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

Fix ingress backend broken SAN #4914

Merged
merged 3 commits into from
Jul 19, 2022
Merged

Fix ingress backend broken SAN #4914

merged 3 commits into from
Jul 19, 2022

Conversation

steeling
Copy link
Contributor

Fix an issue where the trust domain is getting added to the full SAN provided in ingress backend's. This is accomplished by switching from adding service identity (which does not contain trust domain) to principals (which does), on the traffic policy rules and policy builder.

@steeling steeling changed the title Feature/principals Fix ingress backend broken SAN Jul 19, 2022
@steeling steeling force-pushed the feature/principals branch 3 times, most recently from a9bc9e5 to 74f3755 Compare July 19, 2022 15:28
@steeling steeling marked this pull request as ready for review July 19, 2022 16:02
shashankram
shashankram approved these changes Jul 19, 2022
View reviewed changes
@steeling steeling force-pushed the feature/principals branch from 26b6c33 to 1a6dc9d Compare July 19, 2022 16:39
@codecov-commenter
Copy link

codecov-commenter commented Jul 19, 2022 •
edited
Loading

Codecov Report

Merging #4914 (1a6dc9d) into main (f768f64) will decrease coverage by 0.03%.
The diff coverage is 93.93%.

❗ Current head 1a6dc9d differs from pull request most recent head c9b0508. Consider uploading reports for the commit c9b0508 to get more accurate results

@@            Coverage Diff             @@
##             main    #4914      +/-   ##
==========================================
- Coverage   68.67%   68.63%   -0.04%     
==========================================
  Files         220      220              
  Lines       15944    15941       -3     
==========================================
- Hits        10949    10941       -8     
- Misses       4943     4948       +5     
  Partials       52       52
Flag Coverage Δ
unittests 68.63% <93.93%> (-0.04%) ⬇️

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

Impacted Files Coverage Δ
pkg/identity/types.go 74.07% <0.00%> (-5.93%) ⬇️
pkg/catalog/catalog.go 91.30% <100.00%> (+1.30%) ⬆️
pkg/catalog/inbound_traffic_policies.go 94.90% <100.00%> (ø)
pkg/catalog/ingress.go 96.26% <100.00%> (-0.04%) ⬇️
pkg/envoy/lds/rbac.go 80.35% <100.00%> (-0.68%) ⬇️
pkg/envoy/rbac/policy.go 100.00% <100.00%> (ø)
pkg/envoy/rds/route/rbac.go 92.85% <100.00%> (-0.70%) ⬇️
pkg/trafficpolicy/trafficpolicy.go 96.15% <100.00%> (ø)
pkg/messaging/workqueue.go 89.28% <0.00%> (-10.72%) ⬇️

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 f768f64...c9b0508. Read the comment docs.

@steeling steeling force-pushed the feature/principals branch 2 times, most recently from c9b0508 to e3599ee Compare July 19, 2022 18:53
@keithmattix keithmattix added the blocker/release=v1.2.0 Release blocker for v1.2.0 label Jul 19, 2022
steeling added 2 commits July 19, 2022 17:06
@steeling
Use principal in builders, which contains trust domain vs identity
9f3b005 
Signed-off-by: Sean Teeling <seanteeling@microsoft.com>
@steeling
add ingress backend test
cc41d4e 
Signed-off-by: Sean Teeling <seanteeling@microsoft.com>
@steeling steeling force-pushed the feature/principals branch from e3599ee to 15e00d8 Compare July 19, 2022 21:06
@steeling
use alternate trust domain in ingress backend test
cfdc68d 
Signed-off-by: Sean Teeling <seanteeling@microsoft.com>
@steeling steeling force-pushed the feature/principals branch from 15e00d8 to cfdc68d Compare July 19, 2022 22:06
@steeling steeling merged commit 15e46da into openservicemesh:main Jul 19, 2022
@steeling steeling deleted the feature/principals branch July 19, 2022 22:56
nshankar13 pushed a commit to nshankar13/osm that referenced this pull request Aug 31, 2022
@steeling @nshankar13
Fix ingress backend broken SAN (openservicemesh#4914)
4fbdd7f 
Fix ingress backend SAN's, which were getting the trust domain appended to the provided SAN.
This adds an e2e test to catch that going forward.
This also switches the internal builders to use the principal (trust domain appended) vs the identity (no trust domain)
nshankar13 pushed a commit to nshankar13/osm that referenced this pull request Aug 31, 2022
@steeling @nshankar13
Fix ingress backend broken SAN (openservicemesh#4914)
1493cfa 
Fix ingress backend SAN's, which were getting the trust domain appended to the provided SAN.
This adds an e2e test to catch that going forward.
This also switches the internal builders to use the principal (trust domain appended) vs the identity (no trust domain)
@nshankar13 nshankar13 mentioned this pull request Aug 31, 2022
Merged
nshankar13 pushed a commit to nshankar13/osm that referenced this pull request Aug 31, 2022
@steeling @nshankar13
Fix ingress backend broken SAN (openservicemesh#4914)
cf37976 
Fix ingress backend SAN's, which were getting the trust domain appended to the provided SAN.
This adds an e2e test to catch that going forward.
This also switches the internal builders to use the principal (trust domain appended) vs the identity (no trust domain)

Signed-off-by: nshankar13 <nshankar@microsoft.com>
nshankar13 pushed a commit to nshankar13/osm that referenced this pull request Aug 31, 2022
@steeling @nshankar13
[backport] cherry-pick 15e46da to release-v1.2
af95f3e 
Fix ingress backend broken SAN (openservicemesh#4914)

Fix ingress backend SAN's, which were getting the trust domain appended to the provided SAN.
This adds an e2e test to catch that going forward.
This also switches the internal builders to use the principal (trust domain appended) vs the identity (no trust domain)

Signed-off-by: nshankar13 <nshankar@microsoft.com>
nshankar13 pushed a commit to nshankar13/osm that referenced this pull request Aug 31, 2022
@steeling @nshankar13
[backport] cherry-pick 15e46da to release-v1.2
e71e1fd 
Fix ingress backend broken SAN (openservicemesh#4914)

Fix ingress backend SAN's, which were getting the trust domain appended to the provided SAN.
This adds an e2e test to catch that going forward.
This also switches the internal builders to use the principal (trust domain appended) vs the identity (no trust domain)

Signed-off-by: nshankar13 <nshankar@microsoft.com>

[backport] cherry-pick 961c865 to release-v1.2

fix golints G114 and package-comments (openservicemesh#5037)

golints addressed:
1. G114: Use of net/http serve function that has no support for setting timeouts
2. package-comments
3. removes pkg mesh and moves isValidUUID() to pkg/cli/proxy_get.go

Signed-off-by: Shalier Xia <shalierxia@microsoft.com>
nshankar13 pushed a commit to nshankar13/osm that referenced this pull request Aug 31, 2022
@steeling @nshankar13
[backport] cherry-pick 15e46da to release-v1.2
8c33f33 
Fix ingress backend broken SAN (openservicemesh#4914)

Fix ingress backend SAN's, which were getting the trust domain appended to the provided SAN.
This adds an e2e test to catch that going forward.
This also switches the internal builders to use the principal (trust domain appended) vs the identity (no trust domain)

Signed-off-by: nshankar13 <nshankar@microsoft.com>

[backport] cherry-pick 961c865 to release-v1.2

fix golints G114 and package-comments (openservicemesh#5037)

golints addressed:
1. G114: Use of net/http serve function that has no support for setting timeouts
2. package-comments
3. removes pkg mesh and moves isValidUUID() to pkg/cli/proxy_get.go

Signed-off-by: Shalier Xia <shalierxia@microsoft.com>

[backport] cherry-pick df502a7 to release-v1.2

bump version of go to 1.19 (openservicemesh#4972)

Signed-off-by: Sean Teeling <seanteeling@microsoft.com>
nshankar13 pushed a commit to nshankar13/osm that referenced this pull request Aug 31, 2022
@steeling @nshankar13
[backport] cherry-pick 15e46da to release-v1.2
8772032 
Fix ingress backend broken SAN (openservicemesh#4914)

Fix ingress backend SAN's, which were getting the trust domain appended to the provided SAN.
This adds an e2e test to catch that going forward.
This also switches the internal builders to use the principal (trust domain appended) vs the identity (no trust domain)

[backport] cherry-pick 961c865 to release-v1.2

fix golints G114 and package-comments (openservicemesh#5037)

golints addressed:
1. G114: Use of net/http serve function that has no support for setting timeouts
2. package-comments
3. removes pkg mesh and moves isValidUUID() to pkg/cli/proxy_get.go

Signed-off-by: Shalier Xia <shalierxia@microsoft.com>
nshankar13 pushed a commit to nshankar13/osm that referenced this pull request Aug 31, 2022
@steeling @nshankar13
[backport] cherry-pick 15e46da to release-v1.2
1d50630 
Fix ingress backend broken SAN (openservicemesh#4914)

Fix ingress backend SAN's, which were getting the trust domain appended to the provided SAN.
This adds an e2e test to catch that going forward.
This also switches the internal builders to use the principal (trust domain appended) vs the identity (no trust domain)

[backport] cherry-pick 961c865 to release-v1.2

fix golints G114 and package-comments (openservicemesh#5037)

golints addressed:
1. G114: Use of net/http serve function that has no support for setting timeouts
2. package-comments
3. removes pkg mesh and moves isValidUUID() to pkg/cli/proxy_get.go

Signed-off-by: Shalier Xia <shalierxia@microsoft.com>
Signed-off-by: nshankar13 <nshankar@microsoft.com>
nshankar13 pushed a commit to nshankar13/osm that referenced this pull request Aug 31, 2022
@steeling @nshankar13
[backport] cherry-pick 15e46da to release-v1.2
81a1c6d 
Fix ingress backend broken SAN (openservicemesh#4914)

Fix ingress backend SAN's, which were getting the trust domain appended to the provided SAN.
This adds an e2e test to catch that going forward.
This also switches the internal builders to use the principal (trust domain appended) vs the identity (no trust domain)

[backport] cherry-pick 961c865 to release-v1.2

fix golints G114 and package-comments (openservicemesh#5037)

golints addressed:
1. G114: Use of net/http serve function that has no support for setting timeouts
2. package-comments
3. removes pkg mesh and moves isValidUUID() to pkg/cli/proxy_get.go

Signed-off-by: Shalier Xia <shalierxia@microsoft.com>
Signed-off-by: nshankar13 <nshankar@microsoft.com>
nshankar13 pushed a commit to nshankar13/osm that referenced this pull request Aug 31, 2022
@steeling @nshankar13
[backport] cherry-pick 15e46da to release-v1.2
2dbc3a9 
Fix ingress backend broken SAN (openservicemesh#4914)

Fix ingress backend SAN's, which were getting the trust domain appended to the provided SAN.
This adds an e2e test to catch that going forward.
This also switches the internal builders to use the principal (trust domain appended) vs the identity (no trust domain)
nshankar13 pushed a commit to nshankar13/osm that referenced this pull request Aug 31, 2022
@steeling @nshankar13
[backport] cherry-pick 15e46da to release-v1.2
8ccbe7f 
Fix ingress backend broken SAN (openservicemesh#4914)

Fix ingress backend SAN's, which were getting the trust domain appended to the provided SAN.
This adds an e2e test to catch that going forward.
This also switches the internal builders to use the principal (trust domain appended) vs the identity (no trust domain)

Signed-off-by: nshankar13 <nshankar@microsoft.com>
nshankar13 pushed a commit to nshankar13/osm that referenced this pull request Sep 1, 2022
@steeling @nshankar13
[backport] cherry-pick 15e46da to release-v1.2
cdfc4a0 
Fix ingress backend broken SAN (openservicemesh#4914)

Fix ingress backend SAN's, which were getting the trust domain appended to the provided SAN.
This adds an e2e test to catch that going forward.
This also switches the internal builders to use the principal (trust domain appended) vs the identity (no trust domain)

Signed-off-by: nshankar13 <nshankar@microsoft.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@snehachhabria snehachhabria Awaiting requested review from snehachhabria

@nojnhuh nojnhuh Awaiting requested review from nojnhuh

@draychev draychev Awaiting requested review from draychev

@jaellio jaellio Awaiting requested review from jaellio jaellio is a code owner

@trstringer trstringer Awaiting requested review from trstringer

2 more reviewers

@keithmattix keithmattix keithmattix approved these changes

@shashankram shashankram shashankram approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
blocker/release=v1.2.0 Release blocker for v1.2.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@steeling @codecov-commenter @keithmattix @shashankram