From 7f874c91017eefd12df3d824793ab7e9b0be088e Mon Sep 17 00:00:00 2001 From: Katrina Verey Date: Mon, 27 Feb 2023 18:40:59 -0500 Subject: [PATCH 1/2] Create and update the ApplySet parent object --- .../src/k8s.io/kubectl/pkg/cmd/apply/apply.go | 44 +- .../kubectl/pkg/cmd/apply/apply_test.go | 393 ++++++++++++++++-- .../k8s.io/kubectl/pkg/cmd/apply/applyset.go | 352 ++++++++++++++-- .../kubectl/pkg/cmd/apply/applyset_pruner.go | 7 +- 4 files changed, 722 insertions(+), 74 deletions(-) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/apply/apply.go b/staging/src/k8s.io/kubectl/pkg/cmd/apply/apply.go index c0969dbbc65..1fd6728550d 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/apply/apply.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/apply/apply.go @@ -17,6 +17,7 @@ limitations under the License. package apply import ( + "context" "fmt" "io" "net/http" @@ -38,6 +39,7 @@ import ( "k8s.io/cli-runtime/pkg/resource" "k8s.io/client-go/dynamic" "k8s.io/client-go/util/csaupgrade" + "k8s.io/component-base/version" "k8s.io/klog/v2" "k8s.io/kubectl/pkg/cmd/delete" cmdutil "k8s.io/kubectl/pkg/cmd/util" @@ -300,14 +302,22 @@ func (flags *ApplyFlags) ToOptions(f cmdutil.Factory, cmd *cobra.Command, baseNa var applySet *ApplySet if flags.ApplySetRef != "" { - var applySetNs string + parent, err := ParseApplySetParentRef(flags.ApplySetRef, mapper) + if err != nil { + return nil, fmt.Errorf("invalid parent reference %q: %w", flags.ApplySetRef, err) + } // ApplySet uses the namespace value from the flag, but not from the kubeconfig or defaults - if enforceNamespace { - applySetNs = namespace + // This means the namespace flag is required when using a namespaced parent. + if enforceNamespace && parent.IsNamespaced() { + parent.Namespace = namespace } - if applySet, err = NewApplySet(flags.ApplySetRef, applySetNs, mapper); err != nil { - return nil, err + // TODO: is version.Get() the right thing? Does it work for non-kubectl package consumers? + tooling := ApplySetTooling{name: baseName, version: version.Get().String()} + restClient, err := f.ClientForMapping(parent.RESTMapping) + if err != nil || restClient == nil { + return nil, fmt.Errorf("failed to initialize RESTClient for ApplySet: %w", err) } + applySet = NewApplySet(parent, tooling, mapper, restClient) } if flags.Prune { pruneAllowlist := slice.ToSet(flags.PruneAllowlist, flags.PruneWhitelist) @@ -408,8 +418,7 @@ func (o *ApplyOptions) Validate() error { } else if len(o.PruneResources) > 0 { return fmt.Errorf("--prune-allowlist is incompatible with --applyset") } else { - // TODO: remove this once ApplySet implementation is complete - return fmt.Errorf("--applyset is not yet supported") + klog.Warning("WARNING: --prune --applyset is not fully implemented and does not yet prune any resources.") } } else { if !o.All && o.Selector == "" { @@ -497,6 +506,12 @@ func (o *ApplyOptions) Run() error { if len(infos) == 0 && len(errs) == 0 { return fmt.Errorf("no objects passed to apply") } + + if o.ApplySet != nil { + if err := o.ApplySet.FetchParent(); err != nil { + return err + } + } // Iterate through all objects, applying each one. for _, info := range infos { if err := o.applyOneObject(info); err != nil { @@ -982,6 +997,10 @@ func (o *ApplyOptions) MarkObjectVisited(info *resource.Info) error { return err } o.VisitedUids.Insert(metadata.GetUID()) + + if o.ApplySet != nil { + o.ApplySet.MarkObjectVisited(info.Mapping, info.Namespace) + } return nil } @@ -993,14 +1012,21 @@ func (o *ApplyOptions) MarkObjectVisited(info *resource.Info) error { func (o *ApplyOptions) PrintAndPrunePostProcessor() func() error { return func() error { + ctx := context.TODO() if err := o.printObjects(); err != nil { return err } if o.Prune { if cmdutil.ApplySet.IsEnabled() && o.ApplySet != nil { - p := newApplySetPruner(o) - return p.pruneAll() + pruner := newApplySetPruner(o) + if err := pruner.pruneAll(ctx, o.ApplySet); err != nil { + applySetErr := o.ApplySet.UpdateParent(SetUpdateIncomplete, o.DryRunStrategy, o.ValidationDirective) + return utilerrors.NewAggregate([]error{err, applySetErr}) + } + if err := o.ApplySet.UpdateParent(SetUpdateSuccessful, o.DryRunStrategy, o.ValidationDirective); err != nil { + return fmt.Errorf("apply and prune succeeded, but ApplySet update failed: %w", err) + } } else { p := newPruner(o) return p.pruneAll(o) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/apply/apply_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/apply/apply_test.go index f84416ffcb1..912e3bf7dee 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/apply/apply_test.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/apply/apply_test.go @@ -32,13 +32,11 @@ import ( "github.com/spf13/cobra" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "k8s.io/client-go/tools/clientcmd" - clientcmdapi "k8s.io/client-go/tools/clientcmd/api" - appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" @@ -51,6 +49,8 @@ import ( restclient "k8s.io/client-go/rest" "k8s.io/client-go/rest/fake" testing2 "k8s.io/client-go/testing" + "k8s.io/client-go/tools/clientcmd" + clientcmdapi "k8s.io/client-go/tools/clientcmd/api" "k8s.io/client-go/util/csaupgrade" cmdtesting "k8s.io/kubectl/pkg/cmd/testing" cmdutil "k8s.io/kubectl/pkg/cmd/util" @@ -229,21 +229,13 @@ func TestApplyFlagValidation(t *testing.T) { enableAlphas: []cmdutil.FeatureGate{cmdutil.ApplySet}, expectedErr: "--prune-allowlist is incompatible with --applyset", }, - { - args: [][]string{ - {"prune", "true"}, - {"applyset", "foo"}, - {"namespace", "myNs"}, - }, - enableAlphas: []cmdutil.FeatureGate{cmdutil.ApplySet}, - expectedErr: "--applyset is not yet supported", - }, } for i, test := range tests { t.Run(fmt.Sprintf("case %d", i), func(t *testing.T) { f := cmdtesting.NewTestFactory() defer f.Cleanup() + f.Client = &fake.RESTClient{} cmdtesting.WithAlphaEnvs(test.enableAlphas, t, func(t *testing.T) { cmd := &cobra.Command{} flags := NewApplyFlags(genericclioptions.NewTestIOStreamsDiscard()) @@ -343,6 +335,20 @@ func readReplicationController(t *testing.T, filenameRC string) (string, []byte) return metaAccessor.GetName(), rcBytes } +func readService(t *testing.T, filenameSVC string) (string, []byte) { + svcObj := readServiceFromFile(t, filenameSVC) + metaAccessor, err := meta.Accessor(svcObj) + if err != nil { + t.Fatal(err) + } + svcBytes, err := runtime.Encode(codec, svcObj) + if err != nil { + t.Fatal(err) + } + + return metaAccessor.GetName(), svcBytes +} + func readReplicationControllerFromFile(t *testing.T, filename string) *corev1.ReplicationController { data := readBytesFromFile(t, filename) rc := corev1.ReplicationController{} @@ -2085,7 +2091,7 @@ func TestDontAllowApplyWithPodGeneratedName(t *testing.T) { } func TestApplySetParentValidation(t *testing.T) { - tests := map[string]struct { + for name, test := range map[string]struct { applysetFlag string namespaceFlag string setup func(*testing.T, *cmdtesting.TestFactory) @@ -2166,10 +2172,9 @@ func TestApplySetParentValidation(t *testing.T) { expectParentKind: "Secret", expectErr: "namespace is required to use namespace-scoped ApplySet", }, - } - cmdtesting.WithAlphaEnvs([]cmdutil.FeatureGate{cmdutil.ApplySet}, t, func(t *testing.T) { - for name, test := range tests { - t.Run(name, func(t *testing.T) { + } { + t.Run(name, func(t *testing.T) { + cmdtesting.WithAlphaEnvs([]cmdutil.FeatureGate{cmdutil.ApplySet}, t, func(t *testing.T) { cmd := &cobra.Command{} flags := NewApplyFlags(genericclioptions.NewTestIOStreamsDiscard()) flags.AddFlags(cmd) @@ -2178,6 +2183,7 @@ func TestApplySetParentValidation(t *testing.T) { cmd.Flags().Set("prune", "true") f := cmdtesting.NewTestFactory() defer f.Cleanup() + f.Client = &fake.RESTClient{} var expectedParentNs string if test.namespaceFlag != "" { @@ -2199,25 +2205,24 @@ func TestApplySetParentValidation(t *testing.T) { return } - assert.Equal(t, expectedParentNs, o.ApplySet.ParentRef.Namespace) - assert.Equal(t, test.expectParentKind, o.ApplySet.ParentRef.RESTMapping.GroupVersionKind.Kind) + assert.Equal(t, expectedParentNs, o.ApplySet.parent.Namespace) + assert.Equal(t, test.expectParentKind, o.ApplySet.parent.GroupVersionKind.Kind) err = o.Validate() if test.expectErr != "" { require.EqualError(t, err, test.expectErr) - } else if err.Error() == "--applyset is not yet supported" { - // TODO: remove this when the feature is complete } else { require.NoError(t, err, "Validate error") } }) - } - }) + }) + } } func TestLoadObjects(t *testing.T) { - f := cmdtesting.NewTestFactory() + f := cmdtesting.NewTestFactory().WithNamespace("test") defer f.Cleanup() + f.Client = &fake.RESTClient{} testdirs := []string{"testdata/prune/simple"} for _, testdir := range testdirs { @@ -2235,11 +2240,10 @@ func TestLoadObjects(t *testing.T) { t.Fatalf("unexpected error creating apply options: %v", err) } - // TODO(justinsb): Enable validation once we unblock --applyset - // err = o.Validate() - // if err != nil { - // t.Fatalf("unexpected error from validate: %v", err) - // } + err = o.Validate() + if err != nil { + t.Fatalf("unexpected error from validate: %v", err) + } resources, err := o.GetObjects() if err != nil { @@ -2269,3 +2273,334 @@ func TestLoadObjects(t *testing.T) { }) } } + +func TestApplySetParentManagement(t *testing.T) { + // TODO: replace with cmdtesting.InitTestErrorHandler() when the feature is fully implemented + cmdutil.BehaviorOnFatal(func(s string, i int) { + if s != "error: ApplySet-based pruning is not yet implemented" { + t.Fatalf("unexpected exit %d: %s", i, s) + } + }) + defer cmdutil.DefaultBehaviorOnFatal() + + nameRC, rc := readReplicationController(t, filenameRC) + pathRC := "/namespaces/test/replicationcontrollers/" + nameRC + nameParentSecret := "mySet" + pathSecret := "/namespaces/test/secrets/" + nameParentSecret + + tf := cmdtesting.NewTestFactory().WithNamespace("test") + defer tf.Cleanup() + + serverSideData := map[string][]byte{ + pathRC: rc, + } + + tf.Client = &fake.RESTClient{ + NegotiatedSerializer: resource.UnstructuredPlusDefaultContentConfig().NegotiatedSerializer, + Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) { + switch req.Method { + case "GET": + data, ok := serverSideData[req.URL.Path] + if !ok { + return &http.Response{StatusCode: http.StatusNotFound, Header: cmdtesting.DefaultHeader(), Body: io.NopCloser(bytes.NewReader(nil))}, nil + } + return &http.Response{StatusCode: http.StatusOK, Header: cmdtesting.DefaultHeader(), Body: io.NopCloser(bytes.NewReader(data))}, nil + case "PATCH": + if got := req.Header.Get("Content-Type"); got == string(types.ApplyPatchType) { + // crudely save the patch data as the new object and return it + serverSideData[req.URL.Path], _ = io.ReadAll(req.Body) + return &http.Response{StatusCode: http.StatusOK, Header: cmdtesting.DefaultHeader(), Body: io.NopCloser(bytes.NewReader(serverSideData[req.URL.Path]))}, nil + } else { + t.Fatalf("unexpected content-type: %s\n", got) + return nil, nil + } + default: + t.Fatalf("unexpected request: %#v\n%#v", req.URL, req) + return nil, nil + } + }), + } + + // Initially, the rc 'exists' server side but the svc and applyset secret do not + // This should 'update' the rc and create the secret + ioStreams, _, outbuff, errbuff := genericclioptions.NewTestIOStreams() + cmdtesting.WithAlphaEnvs([]cmdutil.FeatureGate{cmdutil.ApplySet}, t, func(t *testing.T) { + cmd := NewCmdApply("kubectl", tf, ioStreams) + cmd.Flags().Set("filename", filenameRC) + cmd.Flags().Set("server-side", "true") + cmd.Flags().Set("applyset", nameParentSecret) + cmd.Flags().Set("prune", "true") + cmd.Run(cmd, []string{}) + }) + assert.Equal(t, "replicationcontroller/test-rc serverside-applied\n", outbuff.String()) + assert.Equal(t, "", errbuff.String()) + createdSecret, err := yaml.JSONToYAML(serverSideData[pathSecret]) + require.NoError(t, err) + require.Equal(t, `apiVersion: v1 +kind: Secret +metadata: + annotations: + applyset.k8s.io/additional-namespaces: "" + applyset.k8s.io/contains-group-resources: replicationcontrollers + applyset.k8s.io/tooling: kubectl/v0.0.0-master+$Format:%H$ + creationTimestamp: null + labels: + applyset.k8s.io/id: placeholder-todo + name: mySet + namespace: test +`, string(createdSecret)) + + // Next, do an apply that creates a second resource, the svc, and updates the applyset secret + outbuff.Reset() + errbuff.Reset() + cmdtesting.WithAlphaEnvs([]cmdutil.FeatureGate{cmdutil.ApplySet}, t, func(t *testing.T) { + cmd := NewCmdApply("kubectl", tf, ioStreams) + cmd.Flags().Set("filename", filenameRC) + cmd.Flags().Set("filename", filenameSVC) + cmd.Flags().Set("server-side", "true") + cmd.Flags().Set("applyset", nameParentSecret) + cmd.Flags().Set("prune", "true") + cmd.Run(cmd, []string{}) + }) + assert.Equal(t, "replicationcontroller/test-rc serverside-applied\nservice/test-service serverside-applied\n", outbuff.String()) + assert.Equal(t, "", errbuff.String()) + updatedSecret, err := yaml.JSONToYAML(serverSideData[pathSecret]) + require.NoError(t, err) + require.Equal(t, `apiVersion: v1 +kind: Secret +metadata: + annotations: + applyset.k8s.io/additional-namespaces: "" + applyset.k8s.io/contains-group-resources: replicationcontrollers,services + applyset.k8s.io/tooling: kubectl/v0.0.0-master+$Format:%H$ + creationTimestamp: null + labels: + applyset.k8s.io/id: placeholder-todo + name: mySet + namespace: test +`, string(updatedSecret)) + + // Next, do an apply that attempts to remove the rc from the set, but pruning fails + // Both types remain in the ApplySet + // TODO: this case will need to be updated to fail the deletion request once pruning works + outbuff.Reset() + errbuff.Reset() + cmdtesting.WithAlphaEnvs([]cmdutil.FeatureGate{cmdutil.ApplySet}, t, func(t *testing.T) { + cmd := NewCmdApply("kubectl", tf, ioStreams) + cmd.Flags().Set("filename", filenameSVC) + cmd.Flags().Set("server-side", "true") + cmd.Flags().Set("applyset", nameParentSecret) + cmd.Flags().Set("prune", "true") + cmd.Run(cmd, []string{}) + }) + assert.Equal(t, "service/test-service serverside-applied\n", outbuff.String()) + assert.Equal(t, "", errbuff.String()) + updatedSecret, err = yaml.JSONToYAML(serverSideData[pathSecret]) + require.NoError(t, err) + require.Equal(t, `apiVersion: v1 +kind: Secret +metadata: + annotations: + applyset.k8s.io/additional-namespaces: "" + applyset.k8s.io/contains-group-resources: replicationcontrollers,services + applyset.k8s.io/tooling: kubectl/v0.0.0-master+$Format:%H$ + creationTimestamp: null + labels: + applyset.k8s.io/id: placeholder-todo + name: mySet + namespace: test +`, string(updatedSecret)) + + // Finally, do an apply that successfully removes the rc and updates the set + // TODO: add this part once pruning can work +} + +func TestApplySetInvalidLiveParent(t *testing.T) { + nameParentSecret := "mySet" + pathSecret := "/namespaces/test/secrets/" + nameParentSecret + tf := cmdtesting.NewTestFactory().WithNamespace("test") + defer tf.Cleanup() + + type testCase struct { + grsAnnotation string + toolingAnnotation string + idLabel string + expectErr string + } + fakeParentGetterForTest := func(t *testing.T, test testCase) *fake.RESTClient { + return &fake.RESTClient{ + NegotiatedSerializer: resource.UnstructuredPlusDefaultContentConfig().NegotiatedSerializer, + Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) { + if req.Method == "GET" && req.URL.Path == pathSecret { + obj := &metav1.PartialObjectMetadata{ + TypeMeta: metav1.TypeMeta{Kind: "Secret", APIVersion: "v1"}, + ObjectMeta: metav1.ObjectMeta{ + Name: nameParentSecret, + Namespace: "test", + Annotations: make(map[string]string), + Labels: make(map[string]string), + }, + } + if test.grsAnnotation != "" { + obj.ObjectMeta.Annotations[ApplySetGRsAnnotation] = test.grsAnnotation + } + if test.toolingAnnotation != "" { + obj.ObjectMeta.Annotations[ApplySetToolingAnnotation] = test.toolingAnnotation + } + if test.idLabel != "" { + obj.ObjectMeta.Labels[ApplySetParentIDLabel] = test.idLabel + } + data, err := json.Marshal(obj) + require.NoError(t, err) + return &http.Response{StatusCode: http.StatusOK, Header: cmdtesting.DefaultHeader(), Body: io.NopCloser(bytes.NewReader(data))}, nil + } + t.Fatalf("unexpected request to %s:\n%#v", req.URL.Path, req) + return nil, nil + }), + } + } + validIDLabel := "placeholder-todo" + validToolingAnnotation := "kubectl/v1.27.0" + validGrsAnnotation := "deployments.apps,namespaces,secrets" + + for name, test := range map[string]testCase{ + "group-resources annotation is required": { + grsAnnotation: "", + toolingAnnotation: validToolingAnnotation, + idLabel: validIDLabel, + expectErr: "error: parsing ApplySet annotation on \"secrets./mySet\": kubectl requires the \"applyset.k8s.io/contains-group-resources\" annotation to be set on all ApplySet parent objects", + }, + "group-resources annotation should not contain invalid resources": { + grsAnnotation: "does-not-exist", + toolingAnnotation: validToolingAnnotation, + idLabel: validIDLabel, + expectErr: "error: parsing ApplySet annotation on \"secrets./mySet\": invalid group resource in \"applyset.k8s.io/contains-group-resources\" annotation: no matches for /, Resource=does-not-exist", + }, + "tooling annotation is required": { + grsAnnotation: validGrsAnnotation, + toolingAnnotation: "", + idLabel: validIDLabel, + expectErr: "error: ApplySet parent object \"secrets./mySet\" already exists and is missing required annotation \"applyset.k8s.io/tooling\"", + }, + "tooling annotation must have kubectl prefix": { + grsAnnotation: validGrsAnnotation, + toolingAnnotation: "helm/v3", + idLabel: validIDLabel, + expectErr: "error: ApplySet parent object \"secrets./mySet\" already exists and is managed by tooling helm instead of kubectl", + }, + "tooling annotation with invalid prefix with one segment can be parsed": { + grsAnnotation: validGrsAnnotation, + toolingAnnotation: "helm", + idLabel: validIDLabel, + expectErr: "error: ApplySet parent object \"secrets./mySet\" already exists and is managed by tooling helm instead of kubectl", + }, + "tooling annotation with invalid prefix with many segments can be parsed": { + grsAnnotation: validGrsAnnotation, + toolingAnnotation: "example.com/tool/why/v1", + idLabel: validIDLabel, + expectErr: "error: ApplySet parent object \"secrets./mySet\" already exists and is managed by tooling example.com/tool/why instead of kubectl", + }, + "ID label is required": { + grsAnnotation: validGrsAnnotation, + toolingAnnotation: validToolingAnnotation, + idLabel: "", + expectErr: "error: ApplySet parent object \"secrets./mySet\" exists and does not have required label applyset.k8s.io/id", + }, + "ID label must match the ApplySet's real ID": { + grsAnnotation: validGrsAnnotation, + toolingAnnotation: validToolingAnnotation, + idLabel: "somethingelse", + expectErr: fmt.Sprintf("error: ApplySet parent object \"secrets./mySet\" exists and has incorrect value for label \"applyset.k8s.io/id\" (got: somethingelse, want: %s)", validIDLabel), + }, + } { + t.Run(name, func(t *testing.T) { + require.NotEmpty(t, test.expectErr, "invalid test case") + cmdutil.BehaviorOnFatal(func(s string, i int) { + assert.Equal(t, test.expectErr, s) + }) + tf.Client = fakeParentGetterForTest(t, test) + + cmdtesting.WithAlphaEnvs([]cmdutil.FeatureGate{cmdutil.ApplySet}, t, func(t *testing.T) { + ioStreams, _, _, _ := genericclioptions.NewTestIOStreams() + cmd := NewCmdApply("kubectl", tf, ioStreams) + cmd.Flags().Set("filename", filenameSVC) + cmd.Flags().Set("server-side", "true") + cmd.Flags().Set("applyset", nameParentSecret) + cmd.Flags().Set("prune", "true") + cmd.Run(cmd, []string{}) + }) + }) + } +} + +func TestApplySetUpdateConflictsAreRetried(t *testing.T) { + // TODO: replace with cmdtesting.InitTestErrorHandler() when the feature is fully implemented + cmdutil.BehaviorOnFatal(func(s string, i int) { + if s != "error: ApplySet-based pruning is not yet implemented" { + t.Fatalf("unexpected exit %d: %s", i, s) + } + }) + defer cmdutil.DefaultBehaviorOnFatal() + + nameParentSecret := "mySet" + pathSecret := "/namespaces/test/secrets/" + nameParentSecret + secretYaml := `apiVersion: v1 +kind: Secret +metadata: + annotations: + applyset.k8s.io/additional-namespaces: "" + applyset.k8s.io/contains-group-resources: replicationcontrollers + applyset.k8s.io/tooling: kubectl/v0.0.0-master+$Format:%H$ + creationTimestamp: null + labels: + applyset.k8s.io/id: placeholder-todo + name: mySet + namespace: test +` + tf := cmdtesting.NewTestFactory().WithNamespace("test") + defer tf.Cleanup() + applyReturnedConflict := false + appliedWithConflictsForced := false + tf.Client = &fake.RESTClient{ + NegotiatedSerializer: resource.UnstructuredPlusDefaultContentConfig().NegotiatedSerializer, + Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) { + if req.Method == "GET" && req.URL.Path == pathSecret { + data, err := yaml.YAMLToJSON([]byte(secretYaml)) + require.NoError(t, err) + return &http.Response{StatusCode: http.StatusOK, Header: cmdtesting.DefaultHeader(), Body: io.NopCloser(bytes.NewReader(data))}, nil + } + + contentType := req.Header.Get("Content-Type") + forceConflicts := req.URL.Query().Get("force") == "true" + if req.Method == "PATCH" && contentType == string(types.ApplyPatchType) { + // make the ApplySet secret SSA request fail unless conflicts are forced + if req.URL.Path == pathSecret { + if !forceConflicts { + applyReturnedConflict = true + return &http.Response{StatusCode: http.StatusConflict, Header: cmdtesting.DefaultHeader(), Body: io.NopCloser(strings.NewReader("Apply failed with 1 conflict: conflict with \"other\": .metadata.annotations.applyset.k8s.io/contains-group-resources"))}, nil + } + appliedWithConflictsForced = true + } + data, err := io.ReadAll(req.Body) + require.NoError(t, err) + return &http.Response{StatusCode: http.StatusOK, Header: cmdtesting.DefaultHeader(), Body: io.NopCloser(bytes.NewReader(data))}, nil + } + t.Fatalf("unexpected request to %s\n%#v", req.URL.Path, req) + return nil, nil + }), + } + + ioStreams, _, outbuff, errbuff := genericclioptions.NewTestIOStreams() + cmdtesting.WithAlphaEnvs([]cmdutil.FeatureGate{cmdutil.ApplySet}, t, func(t *testing.T) { + cmd := NewCmdApply("kubectl", tf, ioStreams) + cmd.Flags().Set("filename", filenameRC) + cmd.Flags().Set("server-side", "true") + cmd.Flags().Set("applyset", nameParentSecret) + cmd.Flags().Set("prune", "true") + cmd.Run(cmd, []string{}) + }) + assert.Equal(t, "replicationcontroller/test-rc serverside-applied\n", outbuff.String()) + assert.Equal(t, "", errbuff.String()) + assert.Truef(t, applyReturnedConflict, "test did not simulate a conflict scenario") + assert.Truef(t, appliedWithConflictsForced, "conflicts were never forced") +} diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/apply/applyset.go b/staging/src/k8s.io/kubectl/pkg/cmd/apply/applyset.go index 309c28d2cfe..181d254f5b2 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/apply/applyset.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/apply/applyset.go @@ -17,27 +17,83 @@ limitations under the License. package apply import ( + "encoding/json" "fmt" + "sort" "strings" + "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" utilerrors "k8s.io/apimachinery/pkg/util/errors" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/cli-runtime/pkg/resource" + "k8s.io/klog/v2" + cmdutil "k8s.io/kubectl/pkg/cmd/util" +) + +// Label and annotation keys from the ApplySet specification. +// https://git.k8s.io/enhancements/keps/sig-cli/3659-kubectl-apply-prune#design-details-applyset-specification +const ( + // ApplySetToolingAnnotation is the key of the label that indicates which tool is used to manage this ApplySet. + // Tooling should refuse to mutate ApplySets belonging to other tools. + // The value must be in the format /. + // Example value: "kubectl/v1.27" or "helm/v3" or "kpt/v1.0.0" + ApplySetToolingAnnotation = "applyset.k8s.io/tooling" + + // ApplySetAdditionalNamespacesAnnotation annotation extends the scope of the ApplySet beyond the parent + // object's own namespace (if any) to include the listed namespaces. The value is a comma-separated + // list of the names of namespaces other than the parent's namespace in which objects are found + // Example value: "kube-system,ns1,ns2". + ApplySetAdditionalNamespacesAnnotation = "applyset.k8s.io/additional-namespaces" + + // 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-kinds, + // in the fully-qualified name format, i.e. .. + // Example value: "certificates.cert-manager.io,configmaps,deployments.apps,secrets,services" + ApplySetGRsAnnotation = "applyset.k8s.io/contains-group-resources" + + // ApplySetParentIDLabel is the key of the label that makes object an ApplySet parent object. + // Its value MUST be the base64 encoding of the hash of the GKNN of the object it is on, + // in the form base64(sha256(...)), using the URL safe encoding of RFC4648. + ApplySetParentIDLabel = "applyset.k8s.io/id" + + // ApplysetPartOfLabel is the key of the label which indicates that the object is a member of an ApplySet. + // The value of the label MUST match the value of ApplySetParentIDLabel on the parent object. + ApplysetPartOfLabel = "applyset.k8s.io/part-of" ) var defaultApplySetParentGVR = schema.GroupVersionResource{Version: "v1", Resource: "secrets"} // ApplySet tracks the information about an applyset apply/prune type ApplySet struct { - // ParentRef is the reference to the parent object that is used to track the applyset. - ParentRef *ApplySetParentRef + // parent is the reference to the parent object that is used to track the applyset. + parent *ApplySetParent - // resources is the set of all the resources that (might) be part of this applyset. - resources map[schema.GroupVersionResource]struct{} + // toolingID is the value to be used and validated in the applyset.k8s.io/tooling annotation. + toolingID ApplySetTooling - // namespaces is the set of all namespaces that (might) contain objects that are part of this applyset. - namespaces map[string]struct{} + // 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 + + // 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 + + // updatedNamespaces is the set of namespaces that will contain objects in this applyset as of when the current operation completes. + updatedNamespaces sets.Set[string] + + restMapper meta.RESTMapper + + // client is a client specific to the parent's type + client resource.RESTClient } var builtinApplySetParentGVRs = map[schema.GroupVersionResource]bool{ @@ -45,27 +101,44 @@ var builtinApplySetParentGVRs = map[schema.GroupVersionResource]bool{ {Version: "v1", Resource: "configmaps"}: true, } -// ApplySetParentRef stores object and type meta for the parent object that is used to track the applyset. -type ApplySetParentRef struct { - Name string - Namespace string - RESTMapping *meta.RESTMapping +// ApplySetParent stores object and type meta for the parent object that is used to track the applyset. +type ApplySetParent struct { + Name string + Namespace string + *meta.RESTMapping } -// NewApplySet creates a new ApplySet object from a parent reference in the format [RESOURCE][.GROUP]/NAME -func NewApplySet(parentRefStr string, nsFromFlag string, mapper meta.RESTMapper) (*ApplySet, error) { - parent, err := parentRefFromStr(parentRefStr, mapper) - if err != nil { - return nil, fmt.Errorf("invalid parent reference %q: %w", parentRefStr, err) - } - if parent.IsNamespaced() { - parent.Namespace = nsFromFlag - } +func (p ApplySetParent) IsNamespaced() bool { + return p.Scope.Name() == meta.RESTScopeNameNamespace +} + +type ApplySetTooling struct { + name string + version string +} + +func (t ApplySetTooling) String() string { + return fmt.Sprintf("%s/%s", t.name, t.version) +} + +// NewApplySet creates a new ApplySet object tracked by the given parent object. +func NewApplySet(parent *ApplySetParent, tooling ApplySetTooling, mapper meta.RESTMapper, client resource.RESTClient) *ApplySet { return &ApplySet{ - resources: make(map[schema.GroupVersionResource]struct{}), - namespaces: make(map[string]struct{}), - ParentRef: parent, - }, nil + currentResources: make(map[schema.GroupVersionResource]*meta.RESTMapping), + currentNamespaces: make(sets.Set[string]), + updatedResources: make(map[schema.GroupVersionResource]*meta.RESTMapping), + updatedNamespaces: make(sets.Set[string]), + parent: parent, + toolingID: tooling, + restMapper: mapper, + client: client, + } +} + +// ParentRef returns the string representation of the parent object using the same format +// that we expect to receive in the --applyset flag on the CLI. +func (a *ApplySet) ParentRef() string { + return fmt.Sprintf("%s.%s/%s", a.parent.Resource.Resource, a.parent.Resource.Group, a.parent.Name) } // ID is the label value that we are using to identify this applyset. @@ -78,10 +151,10 @@ func (a ApplySet) ID() string { func (a ApplySet) Validate() error { var errors []error // TODO: permit CRDs that have the annotation required by the ApplySet specification - if !builtinApplySetParentGVRs[a.ParentRef.RESTMapping.Resource] { - errors = append(errors, fmt.Errorf("resource %q is not permitted as an ApplySet parent", a.ParentRef.RESTMapping.Resource)) + if !builtinApplySetParentGVRs[a.parent.Resource] { + errors = append(errors, fmt.Errorf("resource %q is not permitted as an ApplySet parent", a.parent.Resource)) } - if a.ParentRef.IsNamespaced() && a.ParentRef.Namespace == "" { + if a.parent.IsNamespaced() && a.parent.Namespace == "" { errors = append(errors, fmt.Errorf("namespace is required to use namespace-scoped ApplySet")) } return utilerrors.NewAggregate(errors) @@ -89,7 +162,7 @@ func (a ApplySet) Validate() error { func (a *ApplySet) LabelsForMember() map[string]string { return map[string]string{ - "applyset.k8s.io/part-of": a.ID(), + ApplysetPartOfLabel: a.ID(), } } @@ -107,7 +180,7 @@ func (a *ApplySet) addLabels(objects []*resource.Info) error { } for k, v := range applysetLabels { if _, found := labels[k]; found { - return fmt.Errorf("applyset label %q already set in input data", k) + return fmt.Errorf("ApplySet label %q already set in input data", k) } labels[k] = v } @@ -117,12 +190,223 @@ func (a *ApplySet) addLabels(objects []*resource.Info) error { return nil } -func (p *ApplySetParentRef) IsNamespaced() bool { - return p.RESTMapping.Scope.Name() == meta.RESTScopeNameNamespace +// MarkObjectVisited keeps track of the applied objects, so that we can update the list of in-use namespaces +// and resouce kinds, and so that we can prune objects not applied. +func (a *ApplySet) MarkObjectVisited(resource *meta.RESTMapping, namespace string) { + a.updatedResources[resource.Resource] = resource + if namespace != "" { + a.updatedNamespaces.Insert(namespace) + } } -// parentRefFromStr creates a new ApplySetParentRef from a parent reference in the format [RESOURCE][.GROUP]/NAME -func parentRefFromStr(parentRefStr string, mapper meta.RESTMapper) (*ApplySetParentRef, error) { +func (a *ApplySet) FetchParent() error { + helper := resource.NewHelper(a.client, a.parent.RESTMapping) + obj, err := helper.Get(a.parent.Namespace, a.parent.Name) + if errors.IsNotFound(err) { + return nil + } else if err != nil { + return fmt.Errorf("failed to fetch ApplySet parent object %q from server: %w", a.ParentRef(), err) + } else if obj == nil { + return fmt.Errorf("failed to fetch ApplySet parent object %q from server", a.ParentRef()) + } + + labels, annotations := getLabelsAndAnnotations(obj) + + toolAnnotation, hasToolAnno := annotations[ApplySetToolingAnnotation] + if !hasToolAnno { + return fmt.Errorf("ApplySet parent object %q already exists and is missing required annotation %q", a.ParentRef(), ApplySetToolingAnnotation) + } + if managedBy := toolingBaseName(toolAnnotation); managedBy != a.toolingID.name { + return fmt.Errorf("ApplySet parent object %q already exists and is managed by tooling %s instead of %s", a.ParentRef(), managedBy, a.toolingID.name) + } + + idLabel, hasIDLabel := labels[ApplySetParentIDLabel] + if !hasIDLabel { + return fmt.Errorf("ApplySet parent object %q exists and does not have required label %s", a.ParentRef(), ApplySetParentIDLabel) + } + if idLabel != a.ID() { + 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 { + // TODO: handle GVRs for now-deleted CRDs + return fmt.Errorf("parsing ApplySet annotation on %q: %w", a.ParentRef(), err) + } + a.currentNamespaces = parseNamespacesAnnotation(annotations) + if a.parent.IsNamespaced() { + a.currentNamespaces.Insert(a.parent.Namespace) + } + return nil +} + +func getLabelsAndAnnotations(obj runtime.Object) (map[string]string, map[string]string) { + accessor := meta.NewAccessor() + annotations := make(map[string]string) + labels := make(map[string]string) + + if data, err := accessor.Annotations(obj); err == nil { + annotations = data + } + if data, err := accessor.Labels(obj); err == nil { + labels = data + } + return labels, annotations +} + +func toolingBaseName(toolAnnotation string) string { + parts := strings.Split(toolAnnotation, "/") + if len(parts) >= 2 { + return strings.Join(parts[:len(parts)-1], "/") + } + return toolAnnotation +} + +func parseResourcesAnnotation(annotations map[string]string, mapper meta.RESTMapper) (map[schema.GroupVersionResource]*meta.RESTMapping, error) { + annotation, ok := annotations[ApplySetGRsAnnotation] + if !ok { + // The spec does not require this annotation. However, 'missing' means 'perform discovery' (as opposed to 'present but empty', which means ' this is an empty set'). + // 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) + } + mappings := make(map[schema.GroupVersionResource]*meta.RESTMapping) + for _, grString := range strings.Split(annotation, ",") { + 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) + } + mapping, err := mapper.RESTMapping(gvk.GroupKind()) + if err != nil { + return nil, fmt.Errorf("could not find kind for resource in %q annotation: %w", ApplySetGRsAnnotation, err) + } + mappings[mapping.Resource] = mapping + } + return mappings, nil +} + +func parseNamespacesAnnotation(annotations map[string]string) sets.Set[string] { + namespaces := make(sets.Set[string]) + annotation, ok := annotations[ApplySetAdditionalNamespacesAnnotation] + if !ok { // this annotation is completely optional + return namespaces + } + for _, ns := range strings.Split(annotation, ",") { + namespaces.Insert(ns) + } + return namespaces +} + +type ApplySetUpdateMode string + +var SetUpdateSuccessful ApplySetUpdateMode = "latest" +var SetUpdateIncomplete ApplySetUpdateMode = "superset" + +func (a *ApplySet) UpdateParent(mode ApplySetUpdateMode, dryRun cmdutil.DryRunStrategy, validation string) error { + data, err := json.Marshal(a.buildParentPatch(mode)) + if err != nil { + return fmt.Errorf("failed to encode patch for ApplySet parent: %w", err) + } + err = serverSideApplyRequest(a, data, dryRun, validation, false) + if err != nil && errors.IsConflict(err) { + // Try again with conflicts forced + klog.Warningf("WARNING: failed to update ApplySet: %s\nApplySet field manager %s should own these fields. Retrying with conflicts forced.", err.Error(), a.FieldManager()) + err = serverSideApplyRequest(a, data, dryRun, validation, true) + } + if err != nil { + return fmt.Errorf("failed to update ApplySet: %w", err) + } + return nil +} + +func serverSideApplyRequest(a *ApplySet, data []byte, dryRun cmdutil.DryRunStrategy, validation string, forceConficts bool) error { + if dryRun == cmdutil.DryRunClient { + return nil + } + helper := resource.NewHelper(a.client, a.parent.RESTMapping). + DryRun(dryRun == cmdutil.DryRunServer). + WithFieldManager(a.FieldManager()). + WithFieldValidation(validation) + + options := metav1.PatchOptions{ + Force: &forceConficts, + } + _, err := helper.Patch( + a.parent.Namespace, + a.parent.Name, + types.ApplyPatchType, + data, + &options, + ) + return err +} + +func (a *ApplySet) buildParentPatch(mode ApplySetUpdateMode) *metav1.PartialObjectMetadata { + var newGRsAnnotation, newNsAnnotation string + switch mode { + case SetUpdateIncomplete: + // 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 := make(map[schema.GroupVersionResource]*meta.RESTMapping) + for versionResource, mapping := range a.currentResources { + grSuperset[versionResource] = mapping + } + for versionResource, mapping := range a.updatedResources { + grSuperset[versionResource] = mapping + } + newGRsAnnotation = generateResourcesAnnotation(grSuperset) + newNsAnnotation = generateNamespacesAnnotation(a.currentNamespaces.Union(a.updatedNamespaces), a.parent.Namespace) + case SetUpdateSuccessful: + newGRsAnnotation = generateResourcesAnnotation(a.updatedResources) + newNsAnnotation = generateNamespacesAnnotation(a.updatedNamespaces, a.parent.Namespace) + } + + return &metav1.PartialObjectMetadata{ + TypeMeta: metav1.TypeMeta{ + Kind: a.parent.GroupVersionKind.Kind, + APIVersion: a.parent.GroupVersionKind.GroupVersion().String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: a.parent.Name, + Namespace: a.parent.Namespace, + Annotations: map[string]string{ + ApplySetToolingAnnotation: a.toolingID.String(), + ApplySetGRsAnnotation: newGRsAnnotation, + ApplySetAdditionalNamespacesAnnotation: newNsAnnotation, + }, + Labels: map[string]string{ + ApplySetParentIDLabel: a.ID(), + }, + }, + } +} + +func generateNamespacesAnnotation(namespaces sets.Set[string], skip string) string { + var ns []string + for n := range namespaces { + if n != "" && n != skip { + ns = append(ns, n) + } + } + sort.Strings(ns) + return strings.Join(ns, ",") +} + +func generateResourcesAnnotation(resources map[schema.GroupVersionResource]*meta.RESTMapping) string { + var grs []string + for gvr := range resources { + grs = append(grs, gvr.GroupResource().String()) + } + sort.Strings(grs) + return strings.Join(grs, ",") +} + +func (a ApplySet) FieldManager() string { + return fmt.Sprintf("%s-applyset-%s", a.toolingID.name, a.ID()) // TODO: validate this choice +} + +// ParseApplySetParentRef creates a new ApplySetParentRef from a parent reference in the format [RESOURCE][.GROUP]/NAME +func ParseApplySetParentRef(parentRefStr string, mapper meta.RESTMapper) (*ApplySetParent, error) { var gvr schema.GroupVersionResource var name string @@ -146,5 +430,5 @@ func parentRefFromStr(parentRefStr string, mapper meta.RESTMapper) (*ApplySetPar if err != nil { return nil, err } - return &ApplySetParentRef{Name: name, RESTMapping: mapping}, nil + return &ApplySetParent{Name: name, RESTMapping: mapping}, nil } diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/apply/applyset_pruner.go b/staging/src/k8s.io/kubectl/pkg/cmd/apply/applyset_pruner.go index d14a8fb6545..2818f2d7772 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/apply/applyset_pruner.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/apply/applyset_pruner.go @@ -16,7 +16,10 @@ limitations under the License. package apply -import "fmt" +import ( + "context" + "fmt" +) type applySetPruner struct { } @@ -25,6 +28,6 @@ func newApplySetPruner(_ *ApplyOptions) *applySetPruner { return &applySetPruner{} } -func (p *applySetPruner) pruneAll() error { +func (p *applySetPruner) pruneAll(_ context.Context, _ *ApplySet) error { return fmt.Errorf("ApplySet-based pruning is not yet implemented") } From 3b0e13482e4459f9cb6a006cac2da76333992efd Mon Sep 17 00:00:00 2001 From: Katrina Verey Date: Fri, 3 Mar 2023 11:50:16 -0500 Subject: [PATCH 2/2] Feedback and linter --- .../src/k8s.io/kubectl/pkg/cmd/apply/apply.go | 28 +++- .../kubectl/pkg/cmd/apply/apply_test.go | 24 +-- .../k8s.io/kubectl/pkg/cmd/apply/applyset.go | 154 ++++++++---------- .../kubectl/pkg/cmd/apply/applyset_pruner.go | 2 +- 4 files changed, 94 insertions(+), 114 deletions(-) diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/apply/apply.go b/staging/src/k8s.io/kubectl/pkg/cmd/apply/apply.go index 1fd6728550d..cd84c1242c8 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/apply/apply.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/apply/apply.go @@ -176,6 +176,8 @@ var ( warningMigrationReapplyFailed = "Warning: failed to re-apply configuration after performing Server-Side Apply migration. This is non-fatal and will be retried next time you apply. Error: %[1]s\n" ) +var ApplySetToolVersion = version.Get().GitVersion + // NewApplyFlags returns a default ApplyFlags func NewApplyFlags(streams genericclioptions.IOStreams) *ApplyFlags { return &ApplyFlags{ @@ -312,7 +314,7 @@ func (flags *ApplyFlags) ToOptions(f cmdutil.Factory, cmd *cobra.Command, baseNa parent.Namespace = namespace } // TODO: is version.Get() the right thing? Does it work for non-kubectl package consumers? - tooling := ApplySetTooling{name: baseName, version: version.Get().String()} + tooling := ApplySetTooling{name: baseName, version: ApplySetToolVersion} restClient, err := f.ClientForMapping(parent.RESTMapping) if err != nil || restClient == nil { return nil, fmt.Errorf("failed to initialize RESTClient for ApplySet: %w", err) @@ -511,6 +513,18 @@ func (o *ApplyOptions) Run() error { if err := o.ApplySet.FetchParent(); err != nil { return err } + // Update the live parent object to the superset of the current and previous resources. + // Doing this before the actual apply and prune operations improves behavior by ensuring + // the live object contains the superset on failure. This may cause the next pruning + // operation to make a larger number of GET requests than strictly necessary, but it prevents + // object leakage from the set. The superset will automatically be reduced to the correct + // set by the next successful operation. + for _, info := range infos { + o.ApplySet.AddResource(info.ResourceMapping(), info.Namespace) + } + if err := o.ApplySet.UpdateParent(UpdateToSuperset, o.DryRunStrategy, o.ValidationDirective); err != nil { + return err + } } // Iterate through all objects, applying each one. for _, info := range infos { @@ -997,10 +1011,6 @@ func (o *ApplyOptions) MarkObjectVisited(info *resource.Info) error { return err } o.VisitedUids.Insert(metadata.GetUID()) - - if o.ApplySet != nil { - o.ApplySet.MarkObjectVisited(info.Mapping, info.Namespace) - } return nil } @@ -1021,10 +1031,12 @@ func (o *ApplyOptions) PrintAndPrunePostProcessor() func() error { if cmdutil.ApplySet.IsEnabled() && o.ApplySet != nil { pruner := newApplySetPruner(o) if err := pruner.pruneAll(ctx, o.ApplySet); err != nil { - applySetErr := o.ApplySet.UpdateParent(SetUpdateIncomplete, o.DryRunStrategy, o.ValidationDirective) - return utilerrors.NewAggregate([]error{err, applySetErr}) + // Do not update the ApplySet. If pruning failed, we want to keep the superset + // of the previous and current resources in the ApplySet, so that the pruning + // step of the next apply will be able to clean up the set correctly. + return err } - if err := o.ApplySet.UpdateParent(SetUpdateSuccessful, o.DryRunStrategy, o.ValidationDirective); err != nil { + if err := o.ApplySet.UpdateParent(UpdateToLatestSet, o.DryRunStrategy, o.ValidationDirective); err != nil { return fmt.Errorf("apply and prune succeeded, but ApplySet update failed: %w", err) } } else { diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/apply/apply_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/apply/apply_test.go index 912e3bf7dee..a608fb661bf 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/apply/apply_test.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/apply/apply_test.go @@ -335,20 +335,6 @@ func readReplicationController(t *testing.T, filenameRC string) (string, []byte) return metaAccessor.GetName(), rcBytes } -func readService(t *testing.T, filenameSVC string) (string, []byte) { - svcObj := readServiceFromFile(t, filenameSVC) - metaAccessor, err := meta.Accessor(svcObj) - if err != nil { - t.Fatal(err) - } - svcBytes, err := runtime.Encode(codec, svcObj) - if err != nil { - t.Fatal(err) - } - - return metaAccessor.GetName(), svcBytes -} - func readReplicationControllerFromFile(t *testing.T, filename string) *corev1.ReplicationController { data := readBytesFromFile(t, filename) rc := corev1.ReplicationController{} @@ -2205,8 +2191,8 @@ func TestApplySetParentValidation(t *testing.T) { return } - assert.Equal(t, expectedParentNs, o.ApplySet.parent.Namespace) - assert.Equal(t, test.expectParentKind, o.ApplySet.parent.GroupVersionKind.Kind) + assert.Equal(t, expectedParentNs, o.ApplySet.parentRef.Namespace) + assert.Equal(t, test.expectParentKind, o.ApplySet.parentRef.GroupVersionKind.Kind) err = o.Validate() if test.expectErr != "" { @@ -2486,19 +2472,19 @@ func TestApplySetInvalidLiveParent(t *testing.T) { grsAnnotation: validGrsAnnotation, toolingAnnotation: "helm/v3", idLabel: validIDLabel, - expectErr: "error: ApplySet parent object \"secrets./mySet\" already exists and is managed by tooling helm instead of kubectl", + expectErr: "error: ApplySet parent object \"secrets./mySet\" already exists and is managed by tooling \"helm\" instead of \"kubectl\"", }, "tooling annotation with invalid prefix with one segment can be parsed": { grsAnnotation: validGrsAnnotation, toolingAnnotation: "helm", idLabel: validIDLabel, - expectErr: "error: ApplySet parent object \"secrets./mySet\" already exists and is managed by tooling helm instead of kubectl", + expectErr: "error: ApplySet parent object \"secrets./mySet\" already exists and is managed by tooling \"helm\" instead of \"kubectl\"", }, "tooling annotation with invalid prefix with many segments can be parsed": { grsAnnotation: validGrsAnnotation, toolingAnnotation: "example.com/tool/why/v1", idLabel: validIDLabel, - expectErr: "error: ApplySet parent object \"secrets./mySet\" already exists and is managed by tooling example.com/tool/why instead of kubectl", + expectErr: "error: ApplySet parent object \"secrets./mySet\" already exists and is managed by tooling \"example.com/tool/why\" instead of \"kubectl\"", }, "ID label is required": { grsAnnotation: validGrsAnnotation, diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/apply/applyset.go b/staging/src/k8s.io/kubectl/pkg/cmd/apply/applyset.go index 181d254f5b2..ded17419295 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/apply/applyset.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/apply/applyset.go @@ -72,8 +72,8 @@ var defaultApplySetParentGVR = schema.GroupVersionResource{Version: "v1", Resour // ApplySet tracks the information about an applyset apply/prune type ApplySet struct { - // parent is the reference to the parent object that is used to track the applyset. - parent *ApplySetParent + // parentRef is a reference to the parent object that is used to track the applyset. + parentRef *ApplySetParentRef // toolingID is the value to be used and validated in the applyset.k8s.io/tooling annotation. toolingID ApplySetTooling @@ -92,7 +92,7 @@ type ApplySet struct { restMapper meta.RESTMapper - // client is a client specific to the parent's type + // client is a client specific to the ApplySet parent object's type client resource.RESTClient } @@ -101,17 +101,23 @@ var builtinApplySetParentGVRs = map[schema.GroupVersionResource]bool{ {Version: "v1", Resource: "configmaps"}: true, } -// ApplySetParent stores object and type meta for the parent object that is used to track the applyset. -type ApplySetParent struct { +// ApplySetParentRef stores object and type meta for the parent object that is used to track the applyset. +type ApplySetParentRef struct { Name string Namespace string *meta.RESTMapping } -func (p ApplySetParent) IsNamespaced() bool { +func (p ApplySetParentRef) IsNamespaced() bool { return p.Scope.Name() == meta.RESTScopeNameNamespace } +// String returns the string representation of the parent object using the same format +// that we expect to receive in the --applyset flag on the CLI. +func (p ApplySetParentRef) String() string { + return fmt.Sprintf("%s.%s/%s", p.Resource.Resource, p.Resource.Group, p.Name) +} + type ApplySetTooling struct { name string version string @@ -122,25 +128,19 @@ func (t ApplySetTooling) String() string { } // NewApplySet creates a new ApplySet object tracked by the given parent object. -func NewApplySet(parent *ApplySetParent, tooling ApplySetTooling, mapper meta.RESTMapper, client resource.RESTClient) *ApplySet { +func NewApplySet(parent *ApplySetParentRef, tooling ApplySetTooling, mapper meta.RESTMapper, client resource.RESTClient) *ApplySet { return &ApplySet{ currentResources: make(map[schema.GroupVersionResource]*meta.RESTMapping), currentNamespaces: make(sets.Set[string]), updatedResources: make(map[schema.GroupVersionResource]*meta.RESTMapping), updatedNamespaces: make(sets.Set[string]), - parent: parent, + parentRef: parent, toolingID: tooling, restMapper: mapper, client: client, } } -// ParentRef returns the string representation of the parent object using the same format -// that we expect to receive in the --applyset flag on the CLI. -func (a *ApplySet) ParentRef() string { - return fmt.Sprintf("%s.%s/%s", a.parent.Resource.Resource, a.parent.Resource.Group, a.parent.Name) -} - // ID is the label value that we are using to identify this applyset. func (a ApplySet) ID() string { // TODO: base64(sha256(gknn)) @@ -151,10 +151,10 @@ func (a ApplySet) ID() string { func (a ApplySet) Validate() error { var errors []error // TODO: permit CRDs that have the annotation required by the ApplySet specification - if !builtinApplySetParentGVRs[a.parent.Resource] { - errors = append(errors, fmt.Errorf("resource %q is not permitted as an ApplySet parent", a.parent.Resource)) + if !builtinApplySetParentGVRs[a.parentRef.Resource] { + errors = append(errors, fmt.Errorf("resource %q is not permitted as an ApplySet parent", a.parentRef.Resource)) } - if a.parent.IsNamespaced() && a.parent.Namespace == "" { + if a.parentRef.IsNamespaced() && a.parentRef.Namespace == "" { errors = append(errors, fmt.Errorf("namespace is required to use namespace-scoped ApplySet")) } return utilerrors.NewAggregate(errors) @@ -190,67 +190,55 @@ func (a *ApplySet) addLabels(objects []*resource.Info) error { return nil } -// MarkObjectVisited keeps track of the applied objects, so that we can update the list of in-use namespaces -// and resouce kinds, and so that we can prune objects not applied. -func (a *ApplySet) MarkObjectVisited(resource *meta.RESTMapping, namespace string) { - a.updatedResources[resource.Resource] = resource - if namespace != "" { - a.updatedNamespaces.Insert(namespace) - } -} - func (a *ApplySet) FetchParent() error { - helper := resource.NewHelper(a.client, a.parent.RESTMapping) - obj, err := helper.Get(a.parent.Namespace, a.parent.Name) + helper := resource.NewHelper(a.client, a.parentRef.RESTMapping) + obj, err := helper.Get(a.parentRef.Namespace, a.parentRef.Name) if errors.IsNotFound(err) { return nil } else if err != nil { - return fmt.Errorf("failed to fetch ApplySet parent object %q from server: %w", a.ParentRef(), err) + return fmt.Errorf("failed to fetch ApplySet parent object %q from server: %w", a.parentRef, err) } else if obj == nil { - return fmt.Errorf("failed to fetch ApplySet parent object %q from server", a.ParentRef()) + return fmt.Errorf("failed to fetch ApplySet parent object %q from server", a.parentRef) } - labels, annotations := getLabelsAndAnnotations(obj) + labels, annotations, err := getLabelsAndAnnotations(obj) + if err != nil { + return fmt.Errorf("getting metadata from parent object %q: %w", a.parentRef, err) + } toolAnnotation, hasToolAnno := annotations[ApplySetToolingAnnotation] if !hasToolAnno { - return fmt.Errorf("ApplySet parent object %q already exists and is missing required annotation %q", a.ParentRef(), ApplySetToolingAnnotation) + return fmt.Errorf("ApplySet parent object %q already exists and is missing required annotation %q", a.parentRef, ApplySetToolingAnnotation) } if managedBy := toolingBaseName(toolAnnotation); managedBy != a.toolingID.name { - return fmt.Errorf("ApplySet parent object %q already exists and is managed by tooling %s instead of %s", a.ParentRef(), managedBy, a.toolingID.name) + return fmt.Errorf("ApplySet parent object %q already exists and is managed by tooling %q instead of %q", a.parentRef, managedBy, a.toolingID.name) } idLabel, hasIDLabel := labels[ApplySetParentIDLabel] if !hasIDLabel { - return fmt.Errorf("ApplySet parent object %q exists and does not have required label %s", a.ParentRef(), ApplySetParentIDLabel) + return fmt.Errorf("ApplySet parent object %q exists and does not have required label %s", a.parentRef, ApplySetParentIDLabel) } if idLabel != a.ID() { - 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()) + 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 { // TODO: handle GVRs for now-deleted CRDs - return fmt.Errorf("parsing ApplySet annotation on %q: %w", a.ParentRef(), err) + return fmt.Errorf("parsing ApplySet annotation on %q: %w", a.parentRef, err) } a.currentNamespaces = parseNamespacesAnnotation(annotations) - if a.parent.IsNamespaced() { - a.currentNamespaces.Insert(a.parent.Namespace) + if a.parentRef.IsNamespaced() { + a.currentNamespaces.Insert(a.parentRef.Namespace) } return nil } -func getLabelsAndAnnotations(obj runtime.Object) (map[string]string, map[string]string) { - accessor := meta.NewAccessor() - annotations := make(map[string]string) - labels := make(map[string]string) - - if data, err := accessor.Annotations(obj); err == nil { - annotations = data +func getLabelsAndAnnotations(obj runtime.Object) (map[string]string, map[string]string, error) { + accessor, err := meta.Accessor(obj) + if err != nil { + return nil, nil, err } - if data, err := accessor.Labels(obj); err == nil { - labels = data - } - return labels, annotations + return accessor.GetLabels(), accessor.GetAnnotations(), nil } func toolingBaseName(toolAnnotation string) string { @@ -285,21 +273,26 @@ func parseResourcesAnnotation(annotations map[string]string, mapper meta.RESTMap } func parseNamespacesAnnotation(annotations map[string]string) sets.Set[string] { - namespaces := make(sets.Set[string]) annotation, ok := annotations[ApplySetAdditionalNamespacesAnnotation] if !ok { // this annotation is completely optional - return namespaces + return sets.Set[string]{} } - for _, ns := range strings.Split(annotation, ",") { - namespaces.Insert(ns) + return sets.New(strings.Split(annotation, ",")...) +} + +// 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 != "" { + a.updatedNamespaces.Insert(namespace) } - return namespaces } type ApplySetUpdateMode string -var SetUpdateSuccessful ApplySetUpdateMode = "latest" -var SetUpdateIncomplete ApplySetUpdateMode = "superset" +var UpdateToLatestSet ApplySetUpdateMode = "latest" +var UpdateToSuperset ApplySetUpdateMode = "superset" func (a *ApplySet) UpdateParent(mode ApplySetUpdateMode, dryRun cmdutil.DryRunStrategy, validation string) error { data, err := json.Marshal(a.buildParentPatch(mode)) @@ -322,7 +315,7 @@ func serverSideApplyRequest(a *ApplySet, data []byte, dryRun cmdutil.DryRunStrat if dryRun == cmdutil.DryRunClient { return nil } - helper := resource.NewHelper(a.client, a.parent.RESTMapping). + helper := resource.NewHelper(a.client, a.parentRef.RESTMapping). DryRun(dryRun == cmdutil.DryRunServer). WithFieldManager(a.FieldManager()). WithFieldValidation(validation) @@ -331,8 +324,8 @@ func serverSideApplyRequest(a *ApplySet, data []byte, dryRun cmdutil.DryRunStrat Force: &forceConficts, } _, err := helper.Patch( - a.parent.Namespace, - a.parent.Name, + a.parentRef.Namespace, + a.parentRef.Name, types.ApplyPatchType, data, &options, @@ -343,32 +336,26 @@ func serverSideApplyRequest(a *ApplySet, data []byte, dryRun cmdutil.DryRunStrat func (a *ApplySet) buildParentPatch(mode ApplySetUpdateMode) *metav1.PartialObjectMetadata { var newGRsAnnotation, newNsAnnotation string switch mode { - case SetUpdateIncomplete: + 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 := make(map[schema.GroupVersionResource]*meta.RESTMapping) - for versionResource, mapping := range a.currentResources { - grSuperset[versionResource] = mapping - } - for versionResource, mapping := range a.updatedResources { - grSuperset[versionResource] = mapping - } + grSuperset := sets.KeySet(a.currentResources).Union(sets.KeySet(a.updatedResources)) newGRsAnnotation = generateResourcesAnnotation(grSuperset) - newNsAnnotation = generateNamespacesAnnotation(a.currentNamespaces.Union(a.updatedNamespaces), a.parent.Namespace) - case SetUpdateSuccessful: - newGRsAnnotation = generateResourcesAnnotation(a.updatedResources) - newNsAnnotation = generateNamespacesAnnotation(a.updatedNamespaces, a.parent.Namespace) + newNsAnnotation = generateNamespacesAnnotation(a.currentNamespaces.Union(a.updatedNamespaces), a.parentRef.Namespace) + case UpdateToLatestSet: + newGRsAnnotation = generateResourcesAnnotation(sets.KeySet(a.updatedResources)) + newNsAnnotation = generateNamespacesAnnotation(a.updatedNamespaces, a.parentRef.Namespace) } return &metav1.PartialObjectMetadata{ TypeMeta: metav1.TypeMeta{ - Kind: a.parent.GroupVersionKind.Kind, - APIVersion: a.parent.GroupVersionKind.GroupVersion().String(), + Kind: a.parentRef.GroupVersionKind.Kind, + APIVersion: a.parentRef.GroupVersionKind.GroupVersion().String(), }, ObjectMeta: metav1.ObjectMeta{ - Name: a.parent.Name, - Namespace: a.parent.Namespace, + Name: a.parentRef.Name, + Namespace: a.parentRef.Namespace, Annotations: map[string]string{ ApplySetToolingAnnotation: a.toolingID.String(), ApplySetGRsAnnotation: newGRsAnnotation, @@ -382,17 +369,12 @@ func (a *ApplySet) buildParentPatch(mode ApplySetUpdateMode) *metav1.PartialObje } func generateNamespacesAnnotation(namespaces sets.Set[string], skip string) string { - var ns []string - for n := range namespaces { - if n != "" && n != skip { - ns = append(ns, n) - } - } - sort.Strings(ns) - return strings.Join(ns, ",") + nsList := namespaces.Clone().Delete(skip).UnsortedList() + sort.Strings(nsList) + return strings.Join(nsList, ",") } -func generateResourcesAnnotation(resources map[schema.GroupVersionResource]*meta.RESTMapping) string { +func generateResourcesAnnotation(resources sets.Set[schema.GroupVersionResource]) string { var grs []string for gvr := range resources { grs = append(grs, gvr.GroupResource().String()) @@ -406,7 +388,7 @@ func (a ApplySet) FieldManager() string { } // ParseApplySetParentRef creates a new ApplySetParentRef from a parent reference in the format [RESOURCE][.GROUP]/NAME -func ParseApplySetParentRef(parentRefStr string, mapper meta.RESTMapper) (*ApplySetParent, error) { +func ParseApplySetParentRef(parentRefStr string, mapper meta.RESTMapper) (*ApplySetParentRef, error) { var gvr schema.GroupVersionResource var name string @@ -430,5 +412,5 @@ func ParseApplySetParentRef(parentRefStr string, mapper meta.RESTMapper) (*Apply if err != nil { return nil, err } - return &ApplySetParent{Name: name, RESTMapping: mapping}, nil + return &ApplySetParentRef{Name: name, RESTMapping: mapping}, nil } diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/apply/applyset_pruner.go b/staging/src/k8s.io/kubectl/pkg/cmd/apply/applyset_pruner.go index 2818f2d7772..0425600db59 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/apply/applyset_pruner.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/apply/applyset_pruner.go @@ -28,6 +28,6 @@ func newApplySetPruner(_ *ApplyOptions) *applySetPruner { return &applySetPruner{} } -func (p *applySetPruner) pruneAll(_ context.Context, _ *ApplySet) error { +func (p *applySetPruner) pruneAll(context.Context, *ApplySet) error { return fmt.Errorf("ApplySet-based pruning is not yet implemented") }