(bug) drop namespaces RBAC dependency from migration#486
Merged
Conversation
The migrate initContainer (added in v1.11.0 to move per-cluster status from Classifier.status onto ClassifierReport.status) checks each stale/pending entry in the deprecated status arrays by Get-ing that cluster's Namespace, to decide whether the entry refers to a cluster that's since been deleted. classifier-manager-role has never granted get on core namespaces, so any upgrade that still has real data in those arrays (going from pre-v1.11.0 straight to a current release, skipping the intermediate versions that already ran the migration once and cleared the fields) hits: migrating classifier default-classifier: checking namespace <ns>: namespaces "<ns>" is forbidden: ... and the initContainer crash-loops, so classifier-manager never becomes Ready. Instead of granting the new namespaces permission, check whether the cluster the entry actually refers to (SveltosCluster or CAPI Cluster) still exists, via migrationClusterExists. classifier-manager-role already grants get;list;watch on both sveltosclusters and cluster.x-k8s.io/clusters for other reconcilers, so this needs no new RBAC.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The migrate initContainer (added in v1.11.0 to move per-cluster status from Classifier.status onto ClassifierReport.status) checks each stale/pending entry in the deprecated status arrays by Get-ing that cluster's Namespace, to decide whether the entry refers to a cluster that's since been deleted. classifier-manager-role has never granted get on core namespaces, so any upgrade that still has real data in those arrays (going from pre-v1.11.0 straight to a current release, skipping the intermediate versions that already ran the migration once and cleared the fields) hits:
migrating classifier default-classifier: checking namespace : namespaces "" is forbidden: ...
and the initContainer crash-loops, so classifier-manager never becomes Ready.
Instead of granting the new namespaces permission, check whether the cluster the entry actually refers to (SveltosCluster or CAPI Cluster) still exists, via migrationClusterExists. classifier-manager-role already grants get;list;watch on both sveltosclusters and cluster.x-k8s.io/clusters for other reconcilers, so this needs no new RBAC.
Fixes #485