-
Notifications
You must be signed in to change notification settings - Fork 21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add new rancher setting patch in e2e #660
Add new rancher setting patch in e2e #660
Conversation
@alexander-demicev I checked new setting was introduced in rancher-2.9? And we recently bumped it in turtles e2e #658. Should we re-trigger e2e tests once again to test it? |
Signed-off-by: Alexandr Demicev <alexandr.demicev@suse.com>
78fe021
to
0eb2016
Compare
@furkatgofurov7 I rebased the PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deferring on CI, but this looks good
Can you elaborate more why is this setting needed? |
@Danil-Grigorev rancher/rancher#45628 (comment) should give an idea |
@furkatgofurov7 but this does not explain why we test this setting in our suite, does rancher missing this coverage? Since this has to be enabled with turtles on helm chart installation also to be effective for real import scenarios outside of e2e |
@Danil-Grigorev Defaulting to "strict" is a breaking change for anyone using ngrok in the way we're used to, when enabled allows only certificates verified with CAcerts authority provided by I'm not against extending the E2E suite to use "strict" mode for all our suites but it can done later separately. |
I'm confused @alexander-demicev as I was not advocating for using strict mode, see #660 (comment) So if you saying that strict became new default, and by applying it we re-enables use of ngrok, change makes sense to me. (Maybe this needs to be mentioned in PR description?) |
@Danil-Grigorev We will need to provide CA public key for ngrok(or anything else) if using strict, which shouldn't be a problem for us or users but it can be addressed separately |
What this PR does / why we need it:
For testing purposes we need to set agent-tls-mode to system-store. More details on the setting here: rancher/rancher#45684
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes #
Special notes for your reviewer:
Checklist: