feat: add --skip-ca-certificates flag #617
Conversation
There was a problem hiding this comment.
diff should have parity with sync for the most part, but lacks support for this flag:
14:24:24-0700 esenin $ /tmp/bdeck diff -s /tmp/empty.yaml
deleting ca-certificate b368ce04-cacd-4d0c-8811-9c75a5b7fab5
deleting ca-certificate a37fcb6d-a452-42c9-989f-cd5c49c0828f
Summary:
Created: 0
Updated: 0
Deleted: 2
14:24:27-0700 esenin $ /tmp/bdeck sync -s /tmp/empty.yaml --skip-ca-certificates
Summary:
Created: 0
Updated: 0
Deleted: 0
14:24:29-0700 esenin $ /tmp/bdeck diff -s /tmp/empty.yaml --skip-ca-certificates
Error: unknown flag: --skip-ca-certificates
Other than that and the test version question, looks good to go.
|
|
||
| for _, tc := range tests { | ||
| t.Run(tc.name, func(t *testing.T) { | ||
| kong.RunWhenKong(t, ">=2.7.0") |
There was a problem hiding this comment.
Is there a reason that this is gated at 2.7? CA certificates were added much earlier and I'm not aware of any API difference that'd affect the way this flag operates since.
There was a problem hiding this comment.
I did it mostly to avoid having N tests for this, due to schema differences on various version. If you think that's needed/good to have, I'll split the tests per-version
There was a problem hiding this comment.
What are the differences? If the flag functions the same either way and it's just a matter of the input/output objects not being identical across all versions, fine to leave the gate in place as-is, just add a comment to the effect of "ca_certificates first appeared in 1.3, but we limit to 2.7+ here because the schema changed and the entities aren't the same across all versions, even though the skip functionality works the same."
Since CA certificates are 'global' entities in Kong, they cannot be managed on a per-workspace basis, making it hard to be handled declaratively with decK. This introduces a new --skip-ca-certificates to sync/dump/diff/reset to make sure CA certs are ignored when needed.
GGabriele
force-pushed
the
prevent_ca_cert_reset
branch
from
7199cff
to
716c18b
Compare
Ops, I forgot to git add it :\ did it now |
Codecov Report
@@ Coverage Diff @@
## main #617 +/- ##
==========================================
- Coverage 45.56% 45.49% -0.07%
==========================================
Files 74 74
Lines 8450 8462 +12
==========================================
Hits 3850 3850
- Misses 4241 4253 +12
Partials 359 359
Continue to review full report at Codecov.
|
GGabriele
force-pushed
the
prevent_ca_cert_reset
branch
2 times, most recently
from
9ffdb37
to
661d1e9
Compare
GGabriele
force-pushed
the
prevent_ca_cert_reset
branch
from
661d1e9
to
1680a1b
Compare
* feat: add --skip-ca-certificates flag Since CA certificates are 'global' entities in Kong, they cannot be managed on a per-workspace basis, making it hard to be handled declaratively with decK. This introduces a new --skip-ca-certificates to sync/dump/diff/reset to make sure CA certs are ignored when needed. * tests: add integration tests for --skip-ca-certificates
Since CA certificates are 'global' entities in Kong,
they cannot be managed on a per-workspace basis,
making it hard to be handled declaratively with decK.
This introduces a new
--skip-ca-certificatestosync/dump/diff/reset to make sure CA certs are
ignored when needed.