osm icon indicating copy to clipboard operation
osm copied to clipboard

feat(certs): add validating webhook for MeshRootCertificate

Open schristoff opened this issue 2 years ago • 1 comments

Description:

Adds an additional webhook to the validating webhook configuration for resources in the OSM control plane namespace. The new webhook will be used to validate any MeshRootCertificates created in the control plane namespace. The validator for the MRC is responsible for determining if there is an active root certificate rotation, if a status change is allowed, and preventing updates to the provider and trust domain specs. MRC validation is enabled when the binary flag enableMeshRootCertificate is set to true.

The validator performs the following check on each admission request type:

On add:

  • err with empty trust domain
  • accept if state status not set
  • if the state status is set, err if rotation is already occurring. This is when 2 or more MRCs are present in the control plane namespace with states other than error or inactive
  • accept if the state is an accepted state (one of error, inactive, active, validatingRollout, validatingRollbck, issuingRollout, issuingRollback)
    • Note: at this time we do not verify if the state is valid in combination with an existing MRCs state

On update:

  • err if the provider was updated
  • err if the trust domain was updated
  • accept if the state hasn't changed
  • err if the state transition is invalid
  • err if there is a rotation already occurring and the MRC being updated is not the MRC being rolled out or rolled back

At this time we are not performing any validation on delete.

Testing done:

  • CI
  • Demo
    • Manually add MRCs and test the above validation scenarios

Affected area:

Functional Area
Certificate Management [x]

Please answer the following questions with yes/no.

  1. Does this change contain code from or inspired by another project? No

    • Did you notify the maintainers and provide attribution?
  2. Is this a breaking change? No

  3. Has documentation corresponding to this change been updated in the osm-docs repo (if applicable)? No

schristoff avatar Jun 13 '22 17:06 schristoff

Codecov Report

Merging #4811 (241a939) into main (7abf10d) will increase coverage by 0.10%. The diff coverage is 76.04%.

@@            Coverage Diff             @@
##             main    #4811      +/-   ##
==========================================
+ Coverage   68.72%   68.83%   +0.10%     
==========================================
  Files         220      220              
  Lines       15924    16199     +275     
==========================================
+ Hits        10944    11150     +206     
- Misses       4928     4997      +69     
  Partials       52       52              
Flag Coverage Δ
unittests 68.83% <76.04%> (+0.10%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
cmd/osm-controller/osm-controller.go 16.52% <0.00%> (ø)
pkg/validator/validators.go 73.46% <66.94%> (-6.39%) :arrow_down:
pkg/validator/patch.go 88.27% <100.00%> (+4.31%) :arrow_up:
pkg/validator/server.go 81.63% <100.00%> (+1.19%) :arrow_up:
pkg/service/types.go 58.82% <0.00%> (-3.68%) :arrow_down:
pkg/envoy/cds/cluster.go 89.34% <0.00%> (-3.40%) :arrow_down:
pkg/catalog/inbound_traffic_policies.go 92.49% <0.00%> (-2.42%) :arrow_down:
pkg/envoy/lds/inmesh.go 75.23% <0.00%> (-1.04%) :arrow_down:
pkg/envoy/registry/announcement_handlers.go 84.00% <0.00%> (-0.62%) :arrow_down:
pkg/messaging/workqueue.go 100.00% <0.00%> (+10.71%) :arrow_up:
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 7abf10d...241a939. Read the comment docs.

codecov-commenter avatar Jul 27 '22 17:07 codecov-commenter