diff --git a/pkg/cmd/apply/apply_test.go b/pkg/cmd/apply/apply_test.go index 8ef7d7c8..3347e341 100644 --- a/pkg/cmd/apply/apply_test.go +++ b/pkg/cmd/apply/apply_test.go @@ -2403,7 +2403,7 @@ kind: Secret metadata: annotations: applyset.kubernetes.io/additional-namespaces: "" - applyset.kubernetes.io/contains-group-resources: replicationcontrollers + applyset.kubernetes.io/contains-group-kinds: ReplicationController applyset.kubernetes.io/tooling: kubectl/v0.0.0-master+$Format:%H$ creationTimestamp: null labels: @@ -2437,7 +2437,7 @@ kind: Secret metadata: annotations: applyset.kubernetes.io/additional-namespaces: "" - applyset.kubernetes.io/contains-group-resources: replicationcontrollers,services + applyset.kubernetes.io/contains-group-kinds: ReplicationController,Service applyset.kubernetes.io/tooling: kubectl/v0.0.0-master+$Format:%H$ creationTimestamp: null labels: @@ -2472,7 +2472,7 @@ kind: Secret metadata: annotations: applyset.kubernetes.io/additional-namespaces: "" - applyset.kubernetes.io/contains-group-resources: replicationcontrollers,services + applyset.kubernetes.io/contains-group-kinds: ReplicationController,Service applyset.kubernetes.io/tooling: kubectl/v0.0.0-master+$Format:%H$ creationTimestamp: null labels: @@ -2507,7 +2507,7 @@ kind: Secret metadata: annotations: applyset.kubernetes.io/additional-namespaces: "" - applyset.kubernetes.io/contains-group-resources: services + applyset.kubernetes.io/contains-group-kinds: Service applyset.kubernetes.io/tooling: kubectl/v0.0.0-master+$Format:%H$ creationTimestamp: null labels: @@ -2524,60 +2524,60 @@ func TestApplySetInvalidLiveParent(t *testing.T) { defer tf.Cleanup() type testCase struct { - grsAnnotation string + gksAnnotation string toolingAnnotation string idLabel string expectErr string } validIDLabel := "applyset-0eFHV8ySqp7XoShsGvyWFQD3s96yqwHmzc4e0HR1dsY-v1" validToolingAnnotation := "kubectl/v1.27.0" - validGrsAnnotation := "deployments.apps,namespaces,secrets" + validGksAnnotation := "Deployment.apps,Namespace,Secret" for name, test := range map[string]testCase{ "group-resources annotation is required": { - grsAnnotation: "", + gksAnnotation: "", toolingAnnotation: validToolingAnnotation, idLabel: validIDLabel, - expectErr: "error: parsing ApplySet annotation on \"secrets./my-set\": kubectl requires the \"applyset.kubernetes.io/contains-group-resources\" annotation to be set on all ApplySet parent objects", + expectErr: "error: parsing ApplySet annotation on \"secrets./my-set\": kubectl requires the \"applyset.kubernetes.io/contains-group-kinds\" annotation to be set on all ApplySet parent objects", }, "group-resources annotation should not contain invalid resources": { - grsAnnotation: "does-not-exist", + gksAnnotation: "does-not-exist", toolingAnnotation: validToolingAnnotation, idLabel: validIDLabel, - expectErr: "error: parsing ApplySet annotation on \"secrets./my-set\": invalid group resource in \"applyset.kubernetes.io/contains-group-resources\" annotation: no matches for /, Resource=does-not-exist", + expectErr: "error: parsing ApplySet annotation on \"secrets./my-set\": could not find mapping for kind in \"applyset.kubernetes.io/contains-group-kinds\" annotation: no matches for kind \"does-not-exist\" in group \"\"", }, "tooling annotation is required": { - grsAnnotation: validGrsAnnotation, + gksAnnotation: validGksAnnotation, toolingAnnotation: "", idLabel: validIDLabel, expectErr: "error: ApplySet parent object \"secrets./my-set\" already exists and is missing required annotation \"applyset.kubernetes.io/tooling\"", }, "tooling annotation must have kubectl prefix": { - grsAnnotation: validGrsAnnotation, + gksAnnotation: validGksAnnotation, toolingAnnotation: "helm/v3", idLabel: validIDLabel, expectErr: "error: ApplySet parent object \"secrets./my-set\" already exists and is managed by tooling \"helm\" instead of \"kubectl\"", }, "tooling annotation with invalid prefix with one segment can be parsed": { - grsAnnotation: validGrsAnnotation, + gksAnnotation: validGksAnnotation, toolingAnnotation: "helm", idLabel: validIDLabel, expectErr: "error: ApplySet parent object \"secrets./my-set\" already exists and is managed by tooling \"helm\" instead of \"kubectl\"", }, "tooling annotation with invalid prefix with many segments can be parsed": { - grsAnnotation: validGrsAnnotation, + gksAnnotation: validGksAnnotation, toolingAnnotation: "example.com/tool/why/v1", idLabel: validIDLabel, expectErr: "error: ApplySet parent object \"secrets./my-set\" already exists and is managed by tooling \"example.com/tool/why\" instead of \"kubectl\"", }, "ID label is required": { - grsAnnotation: validGrsAnnotation, + gksAnnotation: validGksAnnotation, toolingAnnotation: validToolingAnnotation, idLabel: "", expectErr: "error: ApplySet parent object \"secrets./my-set\" exists and does not have required label applyset.kubernetes.io/id", }, "ID label must match the ApplySet's real ID": { - grsAnnotation: validGrsAnnotation, + gksAnnotation: validGksAnnotation, toolingAnnotation: validToolingAnnotation, idLabel: "somethingelse", expectErr: fmt.Sprintf("error: ApplySet parent object \"secrets./my-set\" exists and has incorrect value for label \"applyset.kubernetes.io/id\" (got: somethingelse, want: %s)", validIDLabel), @@ -2596,8 +2596,8 @@ func TestApplySetInvalidLiveParent(t *testing.T) { secret.SetNamespace("test") annotations := make(map[string]string) labels := make(map[string]string) - if test.grsAnnotation != "" { - annotations[ApplySetGRsAnnotation] = test.grsAnnotation + if test.gksAnnotation != "" { + annotations[ApplySetGKsAnnotation] = test.gksAnnotation } if test.toolingAnnotation != "" { annotations[ApplySetToolingAnnotation] = test.toolingAnnotation @@ -2670,7 +2670,7 @@ kind: ApplySet metadata: annotations: applyset.kubernetes.io/additional-namespaces: test - applyset.kubernetes.io/contains-group-resources: replicationcontrollers + applyset.kubernetes.io/contains-group-kinds: ReplicationController applyset.kubernetes.io/tooling: kubectl/v0.0.0-master+$Format:%H$ creationTimestamp: null labels: diff --git a/pkg/cmd/apply/applyset.go b/pkg/cmd/apply/applyset.go index d4bdd588..4fd6dd8e 100644 --- a/pkg/cmd/apply/applyset.go +++ b/pkg/cmd/apply/applyset.go @@ -54,13 +54,22 @@ const ( // Example value: "kube-system,ns1,ns2". ApplySetAdditionalNamespacesAnnotation = "applyset.kubernetes.io/additional-namespaces" - // ApplySetGRsAnnotation is a list of group-resources used to optimize listing of ApplySet member objects. + // Deprecated: ApplySetGRsAnnotation is a list of group-resources used to optimize listing of ApplySet member objects. + // It is optional in the ApplySet specification, as tools can perform discovery or use a different optimization. + // However, it is currently required in kubectl. + // When present, the value of this annotation must be a comma separated list of the group-resources, + // in the fully-qualified name format, i.e. .. + // Example value: "certificates.cert-manager.io,configmaps,deployments.apps,secrets,services" + // Deprecated and replaced by ApplySetGKsAnnotation, support for this can be removed in applyset beta or GA. + DeprecatedApplySetGRsAnnotation = "applyset.kubernetes.io/contains-group-resources" + + // ApplySetGKsAnnotation is a list of group-kinds used to optimize listing of ApplySet member objects. // It is optional in the ApplySet specification, as tools can perform discovery or use a different optimization. // However, it is currently required in kubectl. // When present, the value of this annotation must be a comma separated list of the group-kinds, - // in the fully-qualified name format, i.e. .. - // Example value: "certificates.cert-manager.io,configmaps,deployments.apps,secrets,services" - ApplySetGRsAnnotation = "applyset.kubernetes.io/contains-group-resources" + // in the fully-qualified name format, i.e. .. + // Example value: "Certificate.cert-manager.io,ConfigMap,deployments.apps,Secret,Service" + ApplySetGKsAnnotation = "applyset.kubernetes.io/contains-group-kinds" // ApplySetParentIDLabel is the key of the label that makes object an ApplySet parent object. // Its value MUST use the format specified in V1ApplySetIdFormat below @@ -92,13 +101,13 @@ type ApplySet struct { toolingID ApplySetTooling // currentResources is the set of resources that are part of the sever-side set as of when the current operation started. - currentResources map[schema.GroupVersionResource]*meta.RESTMapping + currentResources map[schema.GroupKind]*kindInfo // currentNamespaces is the set of namespaces that contain objects in this applyset as of when the current operation started. currentNamespaces sets.Set[string] // updatedResources is the set of resources that will be part of the set as of when the current operation completes. - updatedResources map[schema.GroupVersionResource]*meta.RESTMapping + updatedResources map[schema.GroupKind]*kindInfo // updatedNamespaces is the set of namespaces that will contain objects in this applyset as of when the current operation completes. updatedNamespaces sets.Set[string] @@ -143,9 +152,9 @@ func (t ApplySetTooling) String() string { // NewApplySet creates a new ApplySet object tracked by the given parent object. func NewApplySet(parent *ApplySetParentRef, tooling ApplySetTooling, mapper meta.RESTMapper, client resource.RESTClient) *ApplySet { return &ApplySet{ - currentResources: make(map[schema.GroupVersionResource]*meta.RESTMapping), + currentResources: make(map[schema.GroupKind]*kindInfo), currentNamespaces: make(sets.Set[string]), - updatedResources: make(map[schema.GroupVersionResource]*meta.RESTMapping), + updatedResources: make(map[schema.GroupKind]*kindInfo), updatedNamespaces: make(sets.Set[string]), parentRef: parent, toolingID: tooling, @@ -284,7 +293,7 @@ func (a *ApplySet) fetchParent() error { return fmt.Errorf("ApplySet parent object %q exists and has incorrect value for label %q (got: %s, want: %s)", a.parentRef, ApplySetParentIDLabel, idLabel, a.ID()) } - if a.currentResources, err = parseResourcesAnnotation(annotations, a.restMapper); err != nil { + if a.currentResources, err = parseKindAnnotation(annotations, a.restMapper); err != nil { // TODO: handle GVRs for now-deleted CRDs return fmt.Errorf("parsing ApplySet annotation on %q: %w", a.parentRef, err) } @@ -302,8 +311,8 @@ func (a *ApplySet) LabelSelectorForMembers() string { // AllPrunableResources returns the list of all resources that should be considered for pruning. // This is potentially a superset of the resources types that actually contain resources. -func (a *ApplySet) AllPrunableResources() []*meta.RESTMapping { - var ret []*meta.RESTMapping +func (a *ApplySet) AllPrunableResources() []*kindInfo { + var ret []*kindInfo for _, m := range a.currentResources { ret = append(ret, m) } @@ -336,14 +345,43 @@ func toolingBaseName(toolAnnotation string) string { return toolAnnotation } -func parseResourcesAnnotation(annotations map[string]string, mapper meta.RESTMapper) (map[schema.GroupVersionResource]*meta.RESTMapping, error) { - annotation, ok := annotations[ApplySetGRsAnnotation] +// kindInfo holds type information about a particular resource type. +type kindInfo struct { + restMapping *meta.RESTMapping +} + +func parseKindAnnotation(annotations map[string]string, mapper meta.RESTMapper) (map[schema.GroupKind]*kindInfo, error) { + annotation, ok := annotations[ApplySetGKsAnnotation] if !ok { + if annotations[DeprecatedApplySetGRsAnnotation] != "" { + return parseDeprecatedResourceAnnotation(annotations[DeprecatedApplySetGRsAnnotation], mapper) + } + // The spec does not require this annotation. However, 'missing' means 'perform discovery'. // We return an error because we do not currently support dynamic discovery in kubectl apply. - return nil, fmt.Errorf("kubectl requires the %q annotation to be set on all ApplySet parent objects", ApplySetGRsAnnotation) + return nil, fmt.Errorf("kubectl requires the %q annotation to be set on all ApplySet parent objects", ApplySetGKsAnnotation) } - mappings := make(map[schema.GroupVersionResource]*meta.RESTMapping) + mappings := make(map[schema.GroupKind]*kindInfo) + // Annotation present but empty means that this is currently an empty set. + if annotation == "" { + return mappings, nil + } + for _, gkString := range strings.Split(annotation, ",") { + gk := schema.ParseGroupKind(gkString) + restMapping, err := mapper.RESTMapping(gk) + if err != nil { + return nil, fmt.Errorf("could not find mapping for kind in %q annotation: %w", ApplySetGKsAnnotation, err) + } + mappings[gk] = &kindInfo{ + restMapping: restMapping, + } + } + + return mappings, nil +} + +func parseDeprecatedResourceAnnotation(annotation string, mapper meta.RESTMapper) (map[schema.GroupKind]*kindInfo, error) { + mappings := make(map[schema.GroupKind]*kindInfo) // Annotation present but empty means that this is currently an empty set. if annotation == "" { return mappings, nil @@ -352,13 +390,15 @@ func parseResourcesAnnotation(annotations map[string]string, mapper meta.RESTMap gr := schema.ParseGroupResource(grString) gvk, err := mapper.KindFor(gr.WithVersion("")) if err != nil { - return nil, fmt.Errorf("invalid group resource in %q annotation: %w", ApplySetGRsAnnotation, err) + return nil, fmt.Errorf("invalid group resource in %q annotation: %w", DeprecatedApplySetGRsAnnotation, err) } - mapping, err := mapper.RESTMapping(gvk.GroupKind()) + restMapping, err := mapper.RESTMapping(gvk.GroupKind()) if err != nil { - return nil, fmt.Errorf("could not find kind for resource in %q annotation: %w", ApplySetGRsAnnotation, err) + return nil, fmt.Errorf("could not find kind for resource in %q annotation: %w", DeprecatedApplySetGRsAnnotation, err) + } + mappings[gvk.GroupKind()] = &kindInfo{ + restMapping: restMapping, } - mappings[mapping.Resource] = mapping } return mappings, nil } @@ -377,9 +417,14 @@ func parseNamespacesAnnotation(annotations map[string]string) sets.Set[string] { // addResource registers the given resource and namespace as being part of the updated set of // resources being applied by the current operation. -func (a *ApplySet) addResource(resource *meta.RESTMapping, namespace string) { - a.updatedResources[resource.Resource] = resource - if resource.Scope == meta.RESTScopeNamespace && namespace != "" { +func (a *ApplySet) addResource(restMapping *meta.RESTMapping, namespace string) { + gk := restMapping.GroupVersionKind.GroupKind() + if _, found := a.updatedResources[gk]; !found { + a.updatedResources[gk] = &kindInfo{ + restMapping: restMapping, + } + } + if restMapping.Scope == meta.RESTScopeNamespace && namespace != "" { a.updatedNamespaces.Insert(namespace) } } @@ -394,6 +439,8 @@ func (a *ApplySet) updateParent(mode ApplySetUpdateMode, dryRun cmdutil.DryRunSt if err != nil { return fmt.Errorf("failed to encode patch for ApplySet parent: %w", err) } + // Note that because we are using SSA, we will remove any annotations we don't specify, + // which is how we remove the deprecated contains-group-resources annotation. err = serverSideApplyRequest(a, data, dryRun, validation, false) if err != nil && errors.IsConflict(err) { // Try again with conflicts forced @@ -429,17 +476,17 @@ func serverSideApplyRequest(a *ApplySet, data []byte, dryRun cmdutil.DryRunStrat } func (a *ApplySet) buildParentPatch(mode ApplySetUpdateMode) *metav1.PartialObjectMetadata { - var newGRsAnnotation, newNsAnnotation string + var newGKsAnnotation, newNsAnnotation string switch mode { case updateToSuperset: // If the apply succeeded but pruning failed, the set of group resources that // the ApplySet should track is the superset of the previous and current resources. // This ensures that the resources that failed to be pruned are not orphaned from the set. grSuperset := sets.KeySet(a.currentResources).Union(sets.KeySet(a.updatedResources)) - newGRsAnnotation = generateResourcesAnnotation(grSuperset) + newGKsAnnotation = generateKindsAnnotation(grSuperset) newNsAnnotation = generateNamespacesAnnotation(a.currentNamespaces.Union(a.updatedNamespaces), a.parentRef.Namespace) case updateToLatestSet: - newGRsAnnotation = generateResourcesAnnotation(sets.KeySet(a.updatedResources)) + newGKsAnnotation = generateKindsAnnotation(sets.KeySet(a.updatedResources)) newNsAnnotation = generateNamespacesAnnotation(a.updatedNamespaces, a.parentRef.Namespace) } @@ -453,7 +500,7 @@ func (a *ApplySet) buildParentPatch(mode ApplySetUpdateMode) *metav1.PartialObje Namespace: a.parentRef.Namespace, Annotations: map[string]string{ ApplySetToolingAnnotation: a.toolingID.String(), - ApplySetGRsAnnotation: newGRsAnnotation, + ApplySetGKsAnnotation: newGKsAnnotation, ApplySetAdditionalNamespacesAnnotation: newNsAnnotation, }, Labels: map[string]string{ @@ -469,13 +516,13 @@ func generateNamespacesAnnotation(namespaces sets.Set[string], skip string) stri return strings.Join(nsList, ",") } -func generateResourcesAnnotation(resources sets.Set[schema.GroupVersionResource]) string { - var grs []string - for gvr := range resources { - grs = append(grs, gvr.GroupResource().String()) +func generateKindsAnnotation(resources sets.Set[schema.GroupKind]) string { + var gks []string + for gk := range resources { + gks = append(gks, gk.String()) } - sort.Strings(grs) - return strings.Join(grs, ",") + sort.Strings(gks) + return strings.Join(gks, ",") } func (a ApplySet) FieldManager() string { diff --git a/pkg/cmd/apply/applyset_pruner.go b/pkg/cmd/apply/applyset_pruner.go index 9308498d..3c064af3 100644 --- a/pkg/cmd/apply/applyset_pruner.go +++ b/pkg/cmd/apply/applyset_pruner.go @@ -77,27 +77,29 @@ func (a *ApplySet) FindAllObjectsToPrune(ctx context.Context, dynamicClient dyna // We run discovery in parallel, in as many goroutines as priority and fairness will allow // (We don't expect many requests in real-world scenarios - maybe tens, unlikely to be hundreds) - for _, restMapping := range a.AllPrunableResources() { - switch restMapping.Scope.Name() { + for gvk, resource := range a.AllPrunableResources() { + scope := resource.restMapping.Scope + + switch scope.Name() { case meta.RESTScopeNameNamespace: for _, namespace := range a.AllPrunableNamespaces() { if namespace == "" { // Just double-check because otherwise we get cryptic error messages - return nil, fmt.Errorf("unexpectedly encountered empty namespace during prune of namespace-scoped resource %v", restMapping.GroupVersionKind) + return nil, fmt.Errorf("unexpectedly encountered empty namespace during prune of namespace-scoped resource %v", gvk) } tasks = append(tasks, &task{ namespace: namespace, - restMapping: restMapping, + restMapping: resource.restMapping, }) } case meta.RESTScopeNameRoot: tasks = append(tasks, &task{ - restMapping: restMapping, + restMapping: resource.restMapping, }) default: - return nil, fmt.Errorf("unhandled scope %q", restMapping.Scope.Name()) + return nil, fmt.Errorf("unhandled scope %q", scope.Name()) } } diff --git a/testdata/apply/applyset-cr.yaml b/testdata/apply/applyset-cr.yaml index 77ed893c..5c1e03c6 100644 --- a/testdata/apply/applyset-cr.yaml +++ b/testdata/apply/applyset-cr.yaml @@ -4,6 +4,6 @@ metadata: name: my-set annotations: applyset.kubernetes.io/tooling: kubectl/v0.0.0 - applyset.kubernetes.io/contains-group-resources: "" + applyset.kubernetes.io/contains-group-kinds: "" labels: applyset.kubernetes.io/id: applyset-rhp1a-HVAVT_dFgyEygyA1BEB82HPp2o10UiFTpqtAs-v1