From 41d2779805801c73393e5e3caee1cef8f5b6c5be Mon Sep 17 00:00:00 2001 From: justinsb Date: Thu, 7 Oct 2021 07:45:41 -0400 Subject: [PATCH] Use a fixed set of pruning kinds We don't want to prune some kinds that are considered dangerous (CustomResourceDefinitions, which delete all the instances of the CRD) and Namespaces (which delete all the objects in that Namespace). In addition, having a hard-coded list means we don't have to remember to manually specify the kind if we delete an object from the manifest (as long as it's in the known set). We can in future support specifying additional kinds, but we can cross that when we need it. --- ...opeio.example.com-addons-bootstrap_content | 19 +++++++ .../bootstrapchannelbuilder/pruning.go | 50 +++++++++++++++++-- 2 files changed, 64 insertions(+), 5 deletions(-) diff --git a/tests/integration/update_cluster/privatekopeio/data/aws_s3_bucket_object_privatekopeio.example.com-addons-bootstrap_content b/tests/integration/update_cluster/privatekopeio/data/aws_s3_bucket_object_privatekopeio.example.com-addons-bootstrap_content index 9dec74c17d..8467d9a6e5 100644 --- a/tests/integration/update_cluster/privatekopeio/data/aws_s3_bucket_object_privatekopeio.example.com-addons-bootstrap_content +++ b/tests/integration/update_cluster/privatekopeio/data/aws_s3_bucket_object_privatekopeio.example.com-addons-bootstrap_content @@ -58,6 +58,10 @@ spec: name: networking.kope.io prune: kinds: + - kind: ConfigMap + labelSelector: addon.kops.k8s.io/name=networking.kope.io,app.kubernetes.io/managed-by=kops + - kind: Service + labelSelector: addon.kops.k8s.io/name=networking.kope.io,app.kubernetes.io/managed-by=kops - kind: ServiceAccount labelSelector: addon.kops.k8s.io/name=networking.kope.io,app.kubernetes.io/managed-by=kops namespaces: @@ -67,12 +71,27 @@ spec: labelSelector: addon.kops.k8s.io/name=networking.kope.io,app.kubernetes.io/managed-by=kops namespaces: - kube-system + - group: apps + kind: Deployment + labelSelector: addon.kops.k8s.io/name=networking.kope.io,app.kubernetes.io/managed-by=kops + - group: apps + kind: StatefulSet + labelSelector: addon.kops.k8s.io/name=networking.kope.io,app.kubernetes.io/managed-by=kops + - group: policy + kind: PodDisruptionBudget + labelSelector: addon.kops.k8s.io/name=networking.kope.io,app.kubernetes.io/managed-by=kops - group: rbac.authorization.k8s.io kind: ClusterRole labelSelector: addon.kops.k8s.io/name=networking.kope.io,app.kubernetes.io/managed-by=kops - group: rbac.authorization.k8s.io kind: ClusterRoleBinding labelSelector: addon.kops.k8s.io/name=networking.kope.io,app.kubernetes.io/managed-by=kops + - group: rbac.authorization.k8s.io + kind: Role + labelSelector: addon.kops.k8s.io/name=networking.kope.io,app.kubernetes.io/managed-by=kops + - group: rbac.authorization.k8s.io + kind: RoleBinding + labelSelector: addon.kops.k8s.io/name=networking.kope.io,app.kubernetes.io/managed-by=kops selector: role.kubernetes.io/networking: "1" version: 9.99.0 diff --git a/upup/pkg/fi/cloudup/bootstrapchannelbuilder/pruning.go b/upup/pkg/fi/cloudup/bootstrapchannelbuilder/pruning.go index bc1ad72c46..ce44cd77dc 100644 --- a/upup/pkg/fi/cloudup/bootstrapchannelbuilder/pruning.go +++ b/upup/pkg/fi/cloudup/bootstrapchannelbuilder/pruning.go @@ -23,6 +23,8 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/klog/v2" + channelsapi "k8s.io/kops/channels/pkg/api" "k8s.io/kops/pkg/kubemanifest" ) @@ -55,12 +57,42 @@ func (b *BootstrapChannelBuilder) addPruneDirectivesForAddon(addon *Addon) error return fmt.Errorf("error parsing selector %v: %w", selectorMap, err) } + // We always include a set of well-known group kinds, + // so that we prune even if we end up removing something from the manifest. + alwaysPruneGroupKinds := []schema.GroupKind{ + {Group: "", Kind: "ConfigMap"}, + {Group: "", Kind: "Service"}, + {Group: "", Kind: "ServiceAccount"}, + {Group: "apps", Kind: "Deployment"}, + {Group: "apps", Kind: "DaemonSet"}, + {Group: "apps", Kind: "StatefulSet"}, + {Group: "rbac.authorization.k8s.io", Kind: "ClusterRole"}, + {Group: "rbac.authorization.k8s.io", Kind: "ClusterRoleBinding"}, + {Group: "rbac.authorization.k8s.io", Kind: "Role"}, + {Group: "rbac.authorization.k8s.io", Kind: "RoleBinding"}, + {Group: "policy", Kind: "PodDisruptionBudget"}, + } + pruneGroupKind := make(map[schema.GroupKind]bool) + for _, gk := range alwaysPruneGroupKinds { + pruneGroupKind[gk] = true + } + + // In addition, we deliberately exclude a few types that are riskier to delete: + // + // * Namespace: because it deletes anything else that happens to be in the namespace + // + // * CustomResourceDefinition: because it deletes all instances of the CRD + neverPruneGroupKinds := map[schema.GroupKind]bool{ + {Group: "", Kind: "Namespace"}: true, + {Group: "apiextensions.k8s.io", Kind: "CustomResourceDefinition"}: true, + } + + // Parse the manifest; we use this to scope pruning to namespaces objects, err := kubemanifest.LoadObjectsFrom(addon.ManifestData) if err != nil { return fmt.Errorf("failed to parse manifest: %w", err) } - - byGroupKind := make(map[schema.GroupKind][]*kubemanifest.Object) + objectsByGK := make(map[schema.GroupKind][]*kubemanifest.Object) for _, object := range objects { gv, err := schema.ParseGroupVersion(object.APIVersion()) if err != nil || gv.Version == "" { @@ -72,13 +104,21 @@ func (b *BootstrapChannelBuilder) addPruneDirectivesForAddon(addon *Addon) error } gk := gvk.GroupKind() - byGroupKind[gk] = append(byGroupKind[gk], object) + objectsByGK[gk] = append(objectsByGK[gk], object) + + // Warn if there are objects in the manifest that we haven't considered + if !pruneGroupKind[gk] { + if !neverPruneGroupKinds[gk] { + klog.Warningf("manifest includes an object of GroupKind %v, which will not be pruned", gk) + } + } } var groupKinds []schema.GroupKind - for gk := range byGroupKind { + for gk := range pruneGroupKind { groupKinds = append(groupKinds, gk) } + sort.Slice(groupKinds, func(i, j int) bool { if groupKinds[i].Group != groupKinds[j].Group { return groupKinds[i].Group < groupKinds[j].Group @@ -92,7 +132,7 @@ func (b *BootstrapChannelBuilder) addPruneDirectivesForAddon(addon *Addon) error pruneSpec.Kind = gk.Kind namespaces := sets.NewString() - for _, object := range byGroupKind[gk] { + for _, object := range objectsByGK[gk] { namespace := object.GetNamespace() if namespace != "" { namespaces.Insert(namespace)