-
Notifications
You must be signed in to change notification settings - Fork 40
Only pass identity provider mountOption for managed sidecar version >= v1.12.2-gke.0 #520
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
Conversation
622c0d1
to
373d9b7
Compare
Is there a way to test this works before we submit? |
803c7f9
to
d910bfb
Compare
Yeah, updated the PR description field to document my testing method |
Can you please add an e2e test for setting "mountOptions:token-server-identity-provider=https://container.googleapis.com/v1/projects//locations/us-central1/clusters/ssy-e2e" + hostnetwork + private sidecar image works as expected Also, what happens in our logic if the customer sets "token-server-identity-provider=https://container.googleapis.com/v1/projects//locations/us-central1/clusters/ssy-e2e" in mountOptions even though its not a private registry or without hostnetwork enabled. What happens in those cases? |
I would rather not have this in e2e test. For one thing the functionality can only be verified during I/O, but the webhook feature flag is not enabled by default yet. For another, we only want to officially support the managed sidecar. Implementing tests for private images would add unnecessary complexity. We have to retrieve project id, cluster name, container endpoint information, etc. in the test environment. And the benefit is not that significant.
If the sidecar detects a tokenServerIdentity is provided, it will start the token server. |
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
@siyanshen Would be good to have e2e test that @amacaskill mentioned if/when its feasible. We support GCSFuse on private nodes by using the custom sidecar feature. Maybe its also good to test everything on sandbox before cutting release. Other than that, lgtm! |
…upstream-release-1.13 Automated cherry pick of #520: Only pass identity provider mountOption for managed sidecar version >= v1.12.2-gke.0
What type of PR is this?
/kind bug
What this PR does / why we need it:
This PR makes sure latest version of driver is compatible with custom or outdated sidecar images.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Tested:
checked cloud logs
a. When min version is set as v1.12.2-gke.0, log shows the version was not supported.
b. When min version is set as v1.6.x-..., log shows the version was supported.
Confirmed official sidecar image can pass the filter to enjoy the feature: Locally updated webhook to set
--shouldInjectSAVol=true
, ran e2e test with a sidecar image of versioin v1.12.2-gke.0. Confirmed the test Pod and IO test bucket via our hostnetwork support feature.Confirmed private sidecar can use this feature when user adds required mountOption in their pod.
a. Locally updated webhook to set
--shouldInjectSAVol=true
.b. Installed onto my test cluster a custom driver and sidecar.
c. Created a test pod from file
d. Verified that we do not pass the mountOption from driver, but the user pod can successfully IO thanks to the mountOption specified in podSpec. https://screenshot.googleplex.com/9R6sFqrypUhbZ5W
Does this PR introduce a user-facing change?: