Make Workload Identity optional #27189
Conversation
lidizheng
added
area/test
release notes: no
tools/run_tests/xds_k8s_test_driver/framework/test_app/server_app.py
Outdated
Show resolved
Hide resolved
| @@ -18,7 +18,9 @@ spec: | |||
| app: ${deployment_name} | |||
| owner: xds-k8s-interop-test | |||
| spec: | |||
| % if service_account_name: | |||
There was a problem hiding this comment.
This service account it OK, it's just a K8S internal service account.
Instead, do not add this annotation when Workload Identity is enabled.
There was a problem hiding this comment.
In the client/server app, I tried to not explicitly create a service account for the given namespace. Is the K8s service account required for basic tests?
There was a problem hiding this comment.
Ah, OK. Technically K8S Service Account can be used for things other than Security and not directly dependent upon Workload Identity, like RBAC https://kubernetes.io/docs/reference/access-authn-authz/rbac/#service-account-permissions.
So with time this logic of not creating Service Account when Workload Identity disabled may change.
However, I'm OK if you want to keep it as if until then.
There was a problem hiding this comment.
Good to learn that. RBAC might be considered one of the security feature and tested in the security test suite. We are still testing the service account managing code. I would propose to keep it simple now, until the use case presents itself.
…app.py Co-authored-by: Sergii Tkachenko <hi@sergii.org>
Co-authored-by: Sergii Tkachenko <hi@sergii.org>
| @@ -18,7 +18,9 @@ spec: | |||
| app: ${deployment_name} | |||
| owner: xds-k8s-interop-test | |||
| spec: | |||
| % if service_account_name: | |||
There was a problem hiding this comment.
Ah, OK. Technically K8S Service Account can be used for things other than Security and not directly dependent upon Workload Identity, like RBAC https://kubernetes.io/docs/reference/access-authn-authz/rbac/#service-account-permissions.
So with time this logic of not creating Service Account when Workload Identity disabled may change.
However, I'm OK if you want to keep it as if until then.
Part of grpc/grpc#27189 and b/198291728. By disabling the workload identity, we should be able to run tests faster and avoid future IAM policy size issue. Kokoro run: https://fusion2.corp.google.com/invocations/b52b1684-47de-406d-a9f6-644909755f34/targets
Part of grpc/grpc#27189 and b/198291728. By disabling the workload identity, we should be able to run tests faster and avoid future IAM policy size issue. Kokoro run: https://fusion2.corp.google.com/invocations/afb37d8a-0c11-43a5-ad55-f6ead4d680f7/targets
copybara-service
bot
added
the
imported
* Make Workload Identity optional * Update tools/run_tests/xds_k8s_test_driver/framework/test_app/server_app.py Co-authored-by: Sergii Tkachenko <hi@sergii.org> * Update tools/run_tests/xds_k8s_test_driver/framework/xds_k8s_flags.py Co-authored-by: Sergii Tkachenko <hi@sergii.org> * Flip the bool flag naming * Correct the flag help description Co-authored-by: Sergii Tkachenko <hi@sergii.org>
* Make Workload Identity optional * Update tools/run_tests/xds_k8s_test_driver/framework/test_app/server_app.py Co-authored-by: Sergii Tkachenko <hi@sergii.org> * Update tools/run_tests/xds_k8s_test_driver/framework/xds_k8s_flags.py Co-authored-by: Sergii Tkachenko <hi@sergii.org> * Flip the bool flag naming * Correct the flag help description Co-authored-by: Sergii Tkachenko <hi@sergii.org>
This PR adds a flag to opt-out of the Workload Identity feature. However, the Workload Identity is bound with GKE cluster. So, this PR also changes the GKE cluster for url-map tests.
UrlMap tests will create 27 unique namespace for each run, and I suspect there is a certain resource race happen when modifying the binding rules (after all, we are not adding the policy binding as transaction). We are running 4 languages 2 branches and 8 rounds per day, in total 64 runs, equals to 1728 potential policy binding addition per day. If there is a leak, it accumulates much faster than before, hence exposed the problem.
Workload Identity is demanded for security features, and a bonus for other basic tests. Until we have a more concrete solution to solve the policy binding issue, I think the basic tests should opt-out for now.
Runs:
New Kokoro runs: