feat(certs): create MRC on install #4747
Conversation
Codecov Report
@@ Coverage Diff @@
## main #4747 +/- ##
==========================================
+ Coverage 68.96% 69.01% +0.04%
==========================================
Files 227 227
Lines 16454 16512 +58
==========================================
+ Hits 11348 11395 +47
- Misses 5054 5065 +11
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.
|
|
Waiting on #4736 |
jaellio
force-pushed
the
createMRC
branch
4 times, most recently
from
1ce273c to
fca63ec
Compare
jaellio
requested review from
shashankram,
snehachhabria,
nojnhuh,
draychev and
trstringer
as code owners
| presetMeshRootCertificate := presetMeshRootCertificateConfigMap.Data[presetMeshRootCertificateJSONKey] | ||
| presetMeshRootCertificateSpec := configv1alpha2.MeshRootCertificateSpec{} | ||
| err := json.Unmarshal([]byte(presetMeshRootCertificate), &presetMeshRootCertificateSpec) | ||
| if err != nil { |
There was a problem hiding this comment.
this function returns an error, so prefer to return the error vs log.Fatal.
There was a problem hiding this comment.
Good point. I was following the same pattern we have for the MeshConfig, but that makes sense. For my own understanding, should a fatal log only be used when a function doesn't return an error? I would consider this error irrecoverable and I think log.Fatal fits here in that sense.
There was a problem hiding this comment.
Ya I'd say that I try to keep errors opaque. So there's either an error or a warning. A warning might get logged (but not returned), and the caller can decide what to do when the call to the function fails.
ie: an error just means the call failed, the caller decides what to do.
Another rule that's touted a lot is: only panic/exit in package main. All other packages should return errors. The typical exception is convenience functions that begin with Must, ie: text.MustParse
Updates the osm-bootstrap to create a default MeshRootCertificate on osm install. Adds a preset-mesh-root-certificate ConfigMap to the Helm templates. The osm-bootstrap will obtain the MRC spec from the ConfigMap and attempt to create a default MRC. If an MRC already exists with complete state and issuing rotationStage then the osm-bootstrap will not create the default MRC. Additional changes: - adds MeshRootCertificate CRD to uninstall list - adds a tokenSecretName value to the osm.vault values Signed-off-by: jaellio <jaellio@microsoft.com>
Signed-off-by: jaellio <jaellio@microsoft.com>
Signed-off-by: jaellio <jaellio@microsoft.com>
Signed-off-by: jaellio <jaellio@microsoft.com>
jaellio
force-pushed
the
createMRC
branch
2 times, most recently
from
7061984 to
1c3223b
Compare
Signed-off-by: jaellio <jaellio@microsoft.com>
Description:
Updates the osm-bootstrap to create a default MeshRootCertificate
on osm install. Adds a preset-mesh-root-certificate ConfigMap to
the Helm templates. The osm-bootstrap will obtain the MRC spec
from the ConfigMap and attempt to create a default MRC. If an MRC
already exists with a complete state and issuing rotation stage then
the osm-bootstrap will not create the default MRC.
Additional changes:
osm.vault values
root certificate
Resolves #4712
Note: This change does not include creating the Vault token
secret. The MRC created is not used at this point by the OSM
control plane. This will come in a later PR.
Testing done:
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)? No