-
Notifications
You must be signed in to change notification settings - Fork 7
fix: Namespace Sync controller should list no resources when source namespace is empty string #725
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
fix: Namespace Sync controller should list no resources when source namespace is empty string #725
Conversation
c1a962d
to
74c9cf7
Compare
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.
Generally I wonder if we can exit early if the sync controller is enabled (perhaps add an enabled property in the helm chart to enable/disable this) and no namespace is specified (again, can error out in the helm chart if enabled but not specified too).
Sure. I'll do this in a separate PR (#726) |
e2e tests failing, will rebase on main once #736 merges |
83150cb
to
73eaba7
Compare
These e2e passed locally. Not sure what the issue is in CI. |
Previously, the controller listed resources in all namespaces when the source namespace was an empty string.
73eaba7
to
7cabebf
Compare
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.
thanks for writing the test to verify this quickly
🤖 I have created a release *beep* *boop* --- ## 0.11.0 (2024-06-27) <!-- Release notes generated using configuration in .github/release.yaml at main --> ## What's Changed ### Exciting New Features 🎉 * feat: Configure namespace sync in helm chart by @dlipovetsky in #726 * feat: Support CRS for local-path provisioner and add CSI e2e by @jimmidyson in #737 * feat: Support HelmAddon strategy for AWS EBS by @jimmidyson in #732 * feat: Deploy snapshot-controller as separate addon by @jimmidyson in #734 * feat: Update AWS CCM versions and add HelmAddon strategy by @jimmidyson in #748 ### Fixes 🔧 * fix: Namespace Sync controller should list no resources when source namespace is empty string by @dlipovetsky in #725 * fix: Temporarily hard-code supported PC version for Nutanix CSI by @jimmidyson in #751 * fix: skip kubeadm CA file when Secret doesn't have a CA by @dkoshkin in #752 * fix: Correctly report failed deploy of ServiceLoadBalancer by @dlipovetsky in #759 ### Other Changes * build: Tidy up goreleaser config by @jimmidyson in #745 * ci: Fix up image loading for lint-test-helm by @jimmidyson in #746 * refactor: Tidy up Nutanix CSI with consistent apply strategy by @jimmidyson in #733 * test(e2e): Set empty env vars for Nutanix e2e vars by @jimmidyson in #749 * refactor: Use recommended "default" function syntax in helm templates by @dlipovetsky in #750 * refactor: Reusable HelmAddon strategy by @jimmidyson in #735 * test(e2e): Various e2e tests fixes by @jimmidyson in #754 * test(e2e): Correct default helm release names for AWS CCM and EBS CSI by @jimmidyson in #756 **Full Changelog**: v0.10.0...v0.11.0 --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
What problem does this PR solve?:
Previously, the controller listed resources in all namespaces when the source namespace was an empty string. Now, it lists no resources.
The PR adds a test verifying the above. It also adds a test that verifies that an the controller will not match any namespaces when the target namespace label key is an empty string.
Which issue(s) this PR fixes:
Fixes #
How Has This Been Tested?:
Special notes for your reviewer: