Refactor/mrc ca handling #4781
Refactor/mrc ca handling #4781
Conversation
Improve naming & data modeling for CA orchestration Signed-off-by: Keith Mattix II <keithmattix2@gmail.com>
on the Certificate resource Signed-off-by: Keith Mattix II <keithmattix2@gmail.com>
keithmattix
requested review from
shashankram,
snehachhabria,
nojnhuh,
draychev,
jaellio and
trstringer
as code owners
Signed-off-by: Keith Mattix II <keithmattix2@gmail.com>
Signed-off-by: Keith Mattix II <keithmattix2@gmail.com>
Signed-off-by: Keith Mattix II <keithmattix2@gmail.com>
pkg/certificate/providers/config.go
Outdated
| kubeConfig: kubeConfig, | ||
| KeyBitSize: cfg.GetCertKeyBitSize(), | ||
| kubeClient: kubeClient, | ||
| certmanagerClient: certmanagerClient, |
There was a problem hiding this comment.
why does this need a certmanager client if (in most cases) it will never use it? I'm curious on your motivation to change it
There was a problem hiding this comment.
Since we eagerly issue a certificate to retrieve the CA during CertificateManager initialization, the tests started to fail since there's no actual cert-manager client, issuer, etc.
There was a problem hiding this comment.
sounds like that's a symptom of something else. Would you mind reverting, or cutting off a branch/commit that we could look at to see if we can fix the underlying issue without that change?
There was a problem hiding this comment.
Did some work to reorganize everything and managed to stub out the initial certificate issuance completely in tests. Let me know what you think!
Signed-off-by: Keith Mattix II <keithmattix2@gmail.com>
… cert-manager controller for a simple test Signed-off-by: Keith Mattix II <keithmattix2@gmail.com>
Codecov Report
@@ Coverage Diff @@
## main #4781 +/- ##
==========================================
- Coverage 69.04% 68.92% -0.13%
==========================================
Files 225 224 -1
Lines 16365 16414 +49
==========================================
+ Hits 11300 11313 +13
- Misses 5013 5049 +36
Partials 52 52
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Description:
Rename several fields/functions and refactor their usage to optimize readability and correctness. Specifically, create a new field
TrustedCAsto differentiate between a trust context and anIssuingCA. AnIssuingCAis the CA that actually issued the certificate.TrustedCAsrepresent the trust context of acertificate.Certificate's recipient.Testing done:
Existing tests should pass with this change
Affected area:
Please answer the following questions with yes/no.
Does this change contain code from or inspired by another project? no
Is this a breaking change? no
Has documentation corresponding to this change been updated in the osm-docs repo (if applicable)?