Skip to content
This repository was archived by the owner on Jul 11, 2023. It is now read-only.

Set (empty) trust domain on listener builder #4802

Merged
merged 1 commit into from
Jun 9, 2022

Conversation

steeling
Copy link
Contributor

@steeling steeling commented Jun 9, 2022

This PR has 2 main changes:

  1. I set the policy builder's AddPrincipal method to AddIdentity. Following my own advice, the builders should accept inputs in terms of OSM concepts, and return envoy configs. A principal is an envoy concept, and so should be constructed internally, and not leak into its parameters.
  2. I set the trust domain (currently an empty string) on the listenerbuilder so that it is used on the policy builder.

The final PR will be to simultaneously remove the trustDomain from service.Identity, while populating the trust domain within the cert manager.

@steeling steeling force-pushed the feature/set-principal branch 2 times, most recently from 4d60657 to dc08728 Compare June 9, 2022 18:44
@keithmattix
Copy link
Contributor

Is this different from #4782? The names are very similar

@steeling steeling changed the title plumb trust domain so it's ready for use Set (empty) trust domain on listener builder Jun 9, 2022
@steeling
Copy link
Contributor Author

steeling commented Jun 9, 2022

Is this different from #4782? The names are very similar

oops didn't realize that, yes they're different; fixed names now

1. Change the policy builder's AddPrincipal, to AddIdentity so
that the builder accepts OSM constructs as inputs, and returns
envoy constructs.
2. Set the trust domain (currently an empty string) on the
listener builder so that it will be used in the future.

Part of openservicemesh#4754

Signed-off-by: Sean Teeling <[email protected]>
@steeling steeling force-pushed the feature/set-principal branch from dc08728 to 8e7b28b Compare June 9, 2022 22:27
@codecov-commenter
Copy link

Codecov Report

Merging #4802 (8e7b28b) into main (914e8f3) will decrease coverage by 0.06%.
The diff coverage is 80.76%.

@@            Coverage Diff             @@
##             main    #4802      +/-   ##
==========================================
- Coverage   68.62%   68.56%   -0.07%     
==========================================
  Files         223      223              
  Lines       16265    16233      -32     
==========================================
- Hits        11162    11130      -32     
  Misses       5052     5052              
  Partials       51       51              
Flag Coverage Δ
unittests 68.56% <80.76%> (-0.07%) ⬇️

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

Impacted Files Coverage Δ
pkg/certificate/manager.go 90.09% <0.00%> (-0.66%) ⬇️
pkg/certificate/types.go 100.00% <ø> (ø)
pkg/envoy/rbac/policy.go 97.67% <81.81%> (-2.33%) ⬇️
pkg/envoy/lds/rbac.go 81.03% <100.00%> (+0.67%) ⬆️
pkg/envoy/lds/response.go 80.00% <100.00%> (+0.28%) ⬆️
pkg/envoy/rds/route/rbac.go 92.85% <100.00%> (ø)
cmd/osm-controller/osm-controller.go 13.28% <0.00%> (-0.60%) ⬇️
pkg/k8s/types.go 100.00% <0.00%> (ø)
pkg/k8s/client.go 95.81% <0.00%> (+1.73%) ⬆️
... and 2 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 914e8f3...8e7b28b. Read the comment docs.

@shashankram shashankram merged commit 3061b05 into openservicemesh:main Jun 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants