Fix egress mtls origination test #14914

keithmattix · 2024-04-18T14:12:44Z

This should fix the failing test in istio/istio#50040, but we need to make sure we capture the underlying behavior change in a release/upgrade note somewhere.

istio-policy-bot · 2024-04-18T14:12:47Z

😊 Welcome! This is either your first contribution to the Istio documentation repo, or
it's been a while since you've been here. A few things you should know:

You can learn about how we write and maintain documentation, our style guidelines,
and the available web site features by visiting Contributing to the Docs.
In the next few minutes, an automatic preview of your change will be built with
a full copy of the istio.io website. You can find this preview by clicking on
the Details link next to the deploy/netlify entry in the status section of this
page.
We care about quality, so we've put in place a number of checks to ensure our documentation
is top-notch. We do spell checking, sanitize the Markdown, ensure all hyperlinks point to a
valid location, and more. If your PR doesn't pass one of these checks, you'll see a red X in the
lint_istio.io entry in the status section. Click on the Details link to get a list of the
problems with your PR. Fix those problems and push an update; this will automatically re-run the
tests. Hopefully this time everything will be perfect!
Once your changes are accepted and merged into the repository, they will initially show up
on https://preliminary.istio.io. The changes will be published to https://istio.io
the next time we do a major release (which typically happens every 3 months or so).
To publish them sooner, add a cherrypick/release-x.xx label, where x.xx is the current
release of Istio.

Thanks for contributing!

Courtesy of your friendly welcome wagon.

content/en/docs/tasks/traffic-management/egress/egress-gateway-tls-origination/index.md

content/en/docs/tasks/traffic-management/egress/egress-gateway-tls-origination/snips.sh

keithmattix · 2024-04-18T15:44:45Z

@frankbu Any idea why gencheck is failing? I have no diff locally

frankbu · 2024-04-18T15:49:56Z

Seems like a go.mod difference:

diff --git a/go.mod b/go.mod
index 40a84f7a8..d0674f050 100644
--- a/go.mod
+++ b/go.mod
@@ -11,11 +11,8 @@ exclude k8s.io/kubernetes v1.13.0
 replace github.com/imdario/mergo => github.com/imdario/mergo v0.3.5
 
 require (
-	github.com/envoyproxy/go-control-plane v0.12.1-0.20240409154308-6a432fea92ca
 	github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2
 	golang.org/x/sync v0.7.0
-	google.golang.org/genproto/googleapis/rpc v0.0.0-20240401170217-c3f982113cda
-	google.golang.org/grpc v1.63.0
 	istio.io/istio v0.0.0-20240413233931-e1eeb10b4f36
 	k8s.io/apimachinery v0.30.0-rc.2
 	k8s.io/client-go v0.30.0-rc.2
@@ -50,6 +47,7 @@ require (
 	github.com/docker/docker v25.0.5+incompatible // indirect
 	github.com/docker/docker-credential-helpers v0.8.1 // indirect
 	github.com/emicklei/go-restful/v3 v3.11.2 // indirect
+	github.com/envoyproxy/go-control-plane v0.12.1-0.20240409154308-6a432fea92ca // indirect
 	github.com/envoyproxy/protoc-gen-validate v1.0.4 // indirect
 	github.com/evanphx/json-patch v5.9.0+incompatible // indirect
 	github.com/evanphx/json-patch/v5 v5.9.0 // indirect
@@ -190,6 +188,8 @@ require (
 	golang.org/x/tools v0.19.0 // indirect
 	gomodules.xyz/jsonpatch/v2 v2.4.0 // indirect
 	google.golang.org/genproto/googleapis/api v0.0.0-20240401170217-c3f982113cda // indirect
+	google.golang.org/genproto/googleapis/rpc v0.0.0-20240401170217-c3f982113cda // indirect
+	google.golang.org/grpc v1.63.0 // indirect
 	google.golang.org/protobuf v1.33.0 // indirect
 	gopkg.in/inf.v0 v0.9.1 // indirect
 	gopkg.in/ini.v1 v1.67.0 // indirect

You can fix it with:

git apply <(curl -sL "https://gcsweb.istio.io/gcs/istio-prow/pr-logs/pull/istio_istio.io/14914/gencheck_istio.io/1780982059813048320/artifacts/check-clean-repo-diff.patch")

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>

ericvn · 2024-04-18T19:42:27Z

Just a note that the gencheck failure will be fixed my doing a make clean gen. clean is needed to remove the test framework files copied in which will update the go.* slightly.

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>

ericvn · 2024-04-18T21:03:13Z

/test doc.test.profile-demo

flake

kfaseela · 2024-04-19T06:23:54Z

did we get this documented as well?

kfaseela · 2024-04-19T06:25:32Z

content/en/docs/tasks/traffic-management/egress/egress-tls-origination/mtls_test.sh

@@ -21,7 +21,6 @@ set -e
 set -u
 set -o pipefail

-source "content/en/docs/tasks/traffic-management/egress/egress-gateway-tls-origination/snips.sh"


why is this not working anymore? is the exportTo part the only change?

The issue is the the egress gateway mTLS tests used the same setup code but must not use export to. Removing this import breaks the dependency between the workload and gateway mTLS tests

vm-cluster.yaml

zirain · 2024-04-19T06:42:30Z

content/en/docs/tasks/traffic-management/egress/egress-tls-origination/index.md

+      annotations:
+        "networking.istio.io/exportTo": "." # simulate an external service by not exporting outside this namespace


this's the only thing different?

Substantively yes

keithmattix · 2024-04-19T13:30:28Z

did we get this documented as well?

I'm trying to track down where the change occurred and add a release note

ericvn · 2024-04-23T14:53:40Z

I'm trying to track down where the change occurred and add a release note

Did you find/create the release note? I don't think that effort needs to hold this PR out, so once the extraneous file is removed, this is ready for merge?

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>

keithmattix · 2024-04-23T14:59:39Z

@ericvn correct

keithmattix · 2024-04-23T15:04:16Z

The context can be found in istio/istio#50478

keithmattix · 2024-04-23T15:27:18Z

/test doc.test.profile-demo
Flake?

kfaseela · 2024-04-23T15:53:40Z

/retest

ericvn · 2024-04-23T16:37:24Z

Removed the cherry-pick 1.22 label since we don't branch 1.22 until the actual release of 1.22.0. For now, the master branch reflects what will be released in 1.22.0 (and at preliminary.istio.io).

* Sync #14914 fix egress mtls origination test into Chinese * Apply suggestions from code review Co-authored-by: Michael <haifeng.yao@daocloud.io> --------- Co-authored-by: Michael <haifeng.yao@daocloud.io>

keithmattix requested review from a team as code owners April 18, 2024 14:12

istio-testing added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Apr 18, 2024

frankbu reviewed Apr 18, 2024

View reviewed changes

content/en/docs/tasks/traffic-management/egress/egress-gateway-tls-origination/index.md Outdated Show resolved Hide resolved

frankbu reviewed Apr 18, 2024

View reviewed changes

content/en/docs/tasks/traffic-management/egress/egress-gateway-tls-origination/snips.sh Outdated Show resolved Hide resolved

Fix egress mtls origination test

fcf2c96

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>

keithmattix force-pushed the fix-egress-mtls-origination branch from dc88218 to fcf2c96 Compare April 18, 2024 16:10

ericvn mentioned this pull request Apr 18, 2024

Enable egress-tls-origination test #14907

Closed

Refactor the egress snips for tests

6e00231

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>

istio-testing added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Apr 18, 2024

Fix go.mod

2120b92

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>

keithmattix mentioned this pull request Apr 18, 2024

Enable the tasks/traffic-management/egress/egress-tls-origination/mtls_test.sh istio.io test istio/istio#50040

Closed

2 tasks

kfaseela reviewed Apr 19, 2024

View reviewed changes

zirain reviewed Apr 19, 2024

View reviewed changes

vm-cluster.yaml Outdated Show resolved Hide resolved

zirain reviewed Apr 19, 2024

View reviewed changes

Remove unneded change

dbb77ab

Signed-off-by: Keith Mattix II <keithmattix@microsoft.com>

ericvn approved these changes Apr 23, 2024

View reviewed changes

keithmattix added the cherrypick/release-1.22 Set this label on a PR to auto-merge it to the release-1.22 branch label Apr 23, 2024

kfaseela approved these changes Apr 23, 2024

View reviewed changes

ericvn removed the cherrypick/release-1.22 Set this label on a PR to auto-merge it to the release-1.22 branch label Apr 23, 2024

istio-testing merged commit 600676f into istio:master Apr 23, 2024
12 checks passed

wilsonwu added a commit to wilsonwu/istio.io that referenced this pull request Apr 24, 2024

Sync istio#14914 fix egress mtls origination test into Chinese

fa05998

wilsonwu mentioned this pull request Apr 24, 2024

[zh] Sync #14914 fix egress mtls origination test into Chinese #14950

Merged

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix egress mtls origination test #14914

Fix egress mtls origination test #14914

keithmattix commented Apr 18, 2024 •

edited

Loading

istio-policy-bot commented Apr 18, 2024

keithmattix commented Apr 18, 2024

frankbu commented Apr 18, 2024

ericvn commented Apr 18, 2024

ericvn commented Apr 18, 2024

kfaseela commented Apr 19, 2024

kfaseela Apr 19, 2024

keithmattix Apr 19, 2024

zirain Apr 19, 2024

keithmattix Apr 19, 2024

keithmattix commented Apr 19, 2024

ericvn commented Apr 23, 2024

keithmattix commented Apr 23, 2024

keithmattix commented Apr 23, 2024

keithmattix commented Apr 23, 2024

kfaseela commented Apr 23, 2024

ericvn commented Apr 23, 2024

		annotations:
		"networking.istio.io/exportTo": "." # simulate an external service by not exporting outside this namespace

Fix egress mtls origination test #14914

Fix egress mtls origination test #14914

Conversation

keithmattix commented Apr 18, 2024 • edited Loading

istio-policy-bot commented Apr 18, 2024

keithmattix commented Apr 18, 2024

frankbu commented Apr 18, 2024

ericvn commented Apr 18, 2024

ericvn commented Apr 18, 2024

kfaseela commented Apr 19, 2024

kfaseela Apr 19, 2024

Choose a reason for hiding this comment

keithmattix Apr 19, 2024

Choose a reason for hiding this comment

zirain Apr 19, 2024

Choose a reason for hiding this comment

keithmattix Apr 19, 2024

Choose a reason for hiding this comment

keithmattix commented Apr 19, 2024

ericvn commented Apr 23, 2024

keithmattix commented Apr 23, 2024

keithmattix commented Apr 23, 2024

keithmattix commented Apr 23, 2024

kfaseela commented Apr 23, 2024

ericvn commented Apr 23, 2024

keithmattix commented Apr 18, 2024 •

edited

Loading