[Bug-Fix] Invalid gcloud get-credentials argument order may break GKE kubeconfig setup - #265
[Bug-Fix] Invalid gcloud get-credentials argument order may break GKE kubeconfig setup#265ChiragSW wants to merge 1 commit into
Conversation
… kubeconfig setup
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes the unnecessary "--" delimiter from the "gcloud container clusters get-credentials" command in both "post_deploy.py" and "credentials.py", updating the corresponding unit tests to match. The review feedback suggests simplifying the tests by asserting the exact list of command arguments directly rather than manually checking indices and relative order, which improves readability and robustness.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| self.assertNotIn("--", args) | ||
| idx_get = args.index("get-credentials") | ||
| idx_delim = args.index("--") | ||
| idx_cluster = args.index("my-cluster") | ||
| self.assertLess(idx_get, idx_delim) | ||
| self.assertLess(idx_delim, idx_cluster) | ||
| self.assertEqual(idx_cluster, idx_get + 1) | ||
| self.assertGreater(args.index("--zone=us-central1-a"), idx_cluster) | ||
| self.assertGreater(args.index("--project=my-proj"), idx_cluster) |
There was a problem hiding this comment.
Instead of manually finding indices and asserting their relative order, you can assert the exact list of arguments directly. This is much more readable, concise, and robustly verifies the entire command structure.
| self.assertNotIn("--", args) | |
| idx_get = args.index("get-credentials") | |
| idx_delim = args.index("--") | |
| idx_cluster = args.index("my-cluster") | |
| self.assertLess(idx_get, idx_delim) | |
| self.assertLess(idx_delim, idx_cluster) | |
| self.assertEqual(idx_cluster, idx_get + 1) | |
| self.assertGreater(args.index("--zone=us-central1-a"), idx_cluster) | |
| self.assertGreater(args.index("--project=my-proj"), idx_cluster) | |
| self.assertEqual( | |
| args, | |
| [ | |
| "gcloud", | |
| "container", | |
| "clusters", | |
| "get-credentials", | |
| "my-cluster", | |
| "--zone=us-central1-a", | |
| "--project=my-proj", | |
| ], | |
| ) |
| self.assertNotIn("--", args) | ||
| idx_get = args.index("get-credentials") | ||
| idx_delim = args.index("--") | ||
| idx_cluster = args.index("my-cluster") | ||
| self.assertLess(idx_get, idx_delim) | ||
| self.assertLess(idx_delim, idx_cluster) | ||
| self.assertEqual(idx_cluster, idx_get + 1) | ||
| self.assertGreater(args.index("--zone=us-central1-a"), idx_cluster) | ||
| self.assertGreater(args.index("--project=my-proj"), idx_cluster) |
There was a problem hiding this comment.
Instead of manually finding indices and asserting their relative order, you can assert the exact list of arguments directly. This is much more readable, concise, and robustly verifies the entire command structure.
| self.assertNotIn("--", args) | |
| idx_get = args.index("get-credentials") | |
| idx_delim = args.index("--") | |
| idx_cluster = args.index("my-cluster") | |
| self.assertLess(idx_get, idx_delim) | |
| self.assertLess(idx_delim, idx_cluster) | |
| self.assertEqual(idx_cluster, idx_get + 1) | |
| self.assertGreater(args.index("--zone=us-central1-a"), idx_cluster) | |
| self.assertGreater(args.index("--project=my-proj"), idx_cluster) | |
| self.assertEqual( | |
| args, | |
| [ | |
| "gcloud", | |
| "container", | |
| "clusters", | |
| "get-credentials", | |
| "my-cluster", | |
| "--zone=us-central1-a", | |
| "--project=my-proj", | |
| ], | |
| ) |
1 participant
Description
Fixes: #264
Made the small args order fix in both calls and updated the tests that were locking in the delimiter. Removed the delimeter from the command order.
I still cannot setup the cloud setup as there are more issues I am currently facing. Will update with it after a successful setup.
Contributor Agreement
Please check all boxes below before submitting your PR for review: