From 6a31757f45693fec5ea4723bcb405ce4437e31ca Mon Sep 17 00:00:00 2001 From: Katrina Verey Date: Tue, 14 Mar 2023 07:46:16 -0400 Subject: [PATCH] Applyset dry run tests + ID value (#116265) * Test for ApplySet with --dry-run=client|server * Use the real format for ApplySet ID * Incorporate feedback * Adjustments from rebase --- .../src/k8s.io/kubectl/pkg/cmd/apply/apply.go | 3 - .../kubectl/pkg/cmd/apply/apply_test.go | 84 +++++++++++++++++-- .../k8s.io/kubectl/pkg/cmd/apply/applyset.go | 24 ++++-- .../simple/manifest1-expected-getobjects.yaml | 4 +- .../simple/manifest2-expected-getobjects.yaml | 2 +- 5 files changed, 100 insertions(+), 17 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 7b8c9a5d160..1f2b40b5923 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/apply/apply.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/apply/apply.go @@ -313,7 +313,6 @@ func (flags *ApplyFlags) ToOptions(f cmdutil.Factory, cmd *cobra.Command, baseNa if enforceNamespace && parent.IsNamespaced() { parent.Namespace = namespace } - // TODO: is version.Get() the right thing? Does it work for non-kubectl package consumers? tooling := ApplySetTooling{name: baseName, version: ApplySetToolVersion} restClient, err := f.ClientForMapping(parent.RESTMapping) if err != nil || restClient == nil { @@ -419,8 +418,6 @@ func (o *ApplyOptions) Validate() error { return fmt.Errorf("--selector is incompatible with --applyset") } else if len(o.PruneResources) > 0 { return fmt.Errorf("--prune-allowlist is incompatible with --applyset") - } else { - klog.Warning("WARNING: --prune --applyset is not fully implemented and does not yet prune any resources.") } } else { if !o.All && o.Selector == "" { 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 dae318a7367..d34cabb372c 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 @@ -2435,7 +2435,7 @@ metadata: applyset.k8s.io/tooling: kubectl/v0.0.0-master+$Format:%H$ creationTimestamp: null labels: - applyset.k8s.io/id: placeholder-todo + applyset.k8s.io/id: applyset-nqNkDlL072a9O3FBtGMDroXnF18TNtgUetAA6vsaglI-v1 name: mySet namespace: test `, string(createdSecret)) @@ -2465,7 +2465,7 @@ metadata: applyset.k8s.io/tooling: kubectl/v0.0.0-master+$Format:%H$ creationTimestamp: null labels: - applyset.k8s.io/id: placeholder-todo + applyset.k8s.io/id: applyset-nqNkDlL072a9O3FBtGMDroXnF18TNtgUetAA6vsaglI-v1 name: mySet namespace: test `, string(updatedSecret)) @@ -2496,7 +2496,7 @@ metadata: applyset.k8s.io/tooling: kubectl/v0.0.0-master+$Format:%H$ creationTimestamp: null labels: - applyset.k8s.io/id: placeholder-todo + applyset.k8s.io/id: applyset-nqNkDlL072a9O3FBtGMDroXnF18TNtgUetAA6vsaglI-v1 name: mySet namespace: test `, string(updatedSecret)) @@ -2527,7 +2527,7 @@ metadata: applyset.k8s.io/tooling: kubectl/v0.0.0-master+$Format:%H$ creationTimestamp: null labels: - applyset.k8s.io/id: placeholder-todo + applyset.k8s.io/id: applyset-nqNkDlL072a9O3FBtGMDroXnF18TNtgUetAA6vsaglI-v1 name: mySet namespace: test `, string(updatedSecret)) @@ -2577,7 +2577,7 @@ func TestApplySetInvalidLiveParent(t *testing.T) { }), } } - validIDLabel := "placeholder-todo" + validIDLabel := "applyset-nqNkDlL072a9O3FBtGMDroXnF18TNtgUetAA6vsaglI-v1" validToolingAnnotation := "kubectl/v1.27.0" validGrsAnnotation := "deployments.apps,namespaces,secrets" @@ -2866,7 +2866,7 @@ metadata: applyset.k8s.io/tooling: kubectl/v0.0.0-master+$Format:%H$ creationTimestamp: null labels: - applyset.k8s.io/id: placeholder-todo + applyset.k8s.io/id: applyset-nqNkDlL072a9O3FBtGMDroXnF18TNtgUetAA6vsaglI-v1 name: mySet namespace: test ` @@ -3147,3 +3147,75 @@ func fatalNoExit(t *testing.T, ioStreams genericclioptions.IOStreams) func(msg s } } } + +func TestApplySetDryRun(t *testing.T) { + cmdtesting.InitTestErrorHandler(t) + 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() + + // Scenario: the rc 'exists' server side but the applyset secret does not + // In dry run mode, non-dry run patch requests should not be made, and the secret should not be created + serverSideData := map[string][]byte{ + pathRC: rc, + } + fakeDryRunClient := func(t *testing.T, allowPatch bool) *fake.RESTClient { + return &fake.RESTClient{ + NegotiatedSerializer: resource.UnstructuredPlusDefaultContentConfig().NegotiatedSerializer, + Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) { + if req.Method == "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 + } + if req.Method == "PATCH" && allowPatch && req.URL.Query().Get("dryRun") == "All" { + 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 + }), + } + } + + t.Run("server side dry run", func(t *testing.T) { + ioStreams, _, outbuff, _ := genericclioptions.NewTestIOStreams() + tf.Client = fakeDryRunClient(t, true) + 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.Flags().Set("dry-run", "server") + cmd.Run(cmd, []string{}) + }) + assert.Equal(t, "replicationcontroller/test-rc serverside-applied (server dry run)\n", outbuff.String()) + assert.Equal(t, len(serverSideData), 1, "unexpected creation") + require.Nil(t, serverSideData[pathSecret], "secret was created") + }) + + t.Run("client side dry run", func(t *testing.T) { + ioStreams, _, outbuff, _ := genericclioptions.NewTestIOStreams() + tf.Client = fakeDryRunClient(t, false) + cmdtesting.WithAlphaEnvs([]cmdutil.FeatureGate{cmdutil.ApplySet}, t, func(t *testing.T) { + cmd := NewCmdApply("kubectl", tf, ioStreams) + cmd.Flags().Set("filename", filenameRC) + cmd.Flags().Set("applyset", nameParentSecret) + cmd.Flags().Set("prune", "true") + cmd.Flags().Set("dry-run", "client") + cmd.Run(cmd, []string{}) + }) + assert.Equal(t, "replicationcontroller/test-rc configured (dry run)\n", outbuff.String()) + assert.Equal(t, len(serverSideData), 1, "unexpected creation") + require.Nil(t, serverSideData[pathSecret], "secret was created") + }) +} 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 6346d6a57d4..fb6522954cc 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/apply/applyset.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/apply/applyset.go @@ -17,6 +17,8 @@ limitations under the License. package apply import ( + "crypto/sha256" + "encoding/base64" "encoding/json" "fmt" "sort" @@ -59,10 +61,15 @@ const ( 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. + // Its value MUST use the format specified in V1ApplySetIdFormat below ApplySetParentIDLabel = "applyset.k8s.io/id" + // V1ApplySetIdFormat is the format required for the value of ApplySetParentIDLabel (and ApplysetPartOfLabel). + // The %s segment is the unique ID of the object itself, which MUST be the base64 encoding + // (using the URL safe encoding of RFC4648) of the hash of the GKNN of the object it is on, in the form: + // base64(sha256(...)). + V1ApplySetIdFormat = "applyset-%s-v1" + // 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" @@ -141,10 +148,17 @@ func NewApplySet(parent *ApplySetParentRef, tooling ApplySetTooling, mapper meta } } +const applySetIDPartDelimiter = "." + // ID is the label value that we are using to identify this applyset. +// Format: base64(sha256(...)), using the URL safe encoding of RFC4648. + func (a ApplySet) ID() string { - // TODO: base64(sha256(gknn)) - return "placeholder-todo" + unencoded := strings.Join([]string{a.parentRef.Name, a.parentRef.Namespace, a.parentRef.GroupVersionKind.Kind, a.parentRef.GroupVersionKind.Group}, applySetIDPartDelimiter) + hashed := sha256.Sum256([]byte(unencoded)) + b64 := base64.RawURLEncoding.EncodeToString(hashed[:]) + // Label values must start and end with alphanumeric values, so add a known-safe prefix and suffix. + return fmt.Sprintf(V1ApplySetIdFormat, b64) } // Validate imposes restrictions on the parent object that is used to track the applyset. @@ -413,7 +427,7 @@ func generateResourcesAnnotation(resources sets.Set[schema.GroupVersionResource] } func (a ApplySet) FieldManager() string { - return fmt.Sprintf("%s-applyset-%s", a.toolingID.name, a.ID()) // TODO: validate this choice + return fmt.Sprintf("%s-applyset", a.toolingID.name) } // ParseApplySetParentRef creates a new ApplySetParentRef from a parent reference in the format [RESOURCE][.GROUP]/NAME diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/apply/testdata/prune/simple/manifest1-expected-getobjects.yaml b/staging/src/k8s.io/kubectl/pkg/cmd/apply/testdata/prune/simple/manifest1-expected-getobjects.yaml index 6e85b76b74f..1c208928ec6 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/apply/testdata/prune/simple/manifest1-expected-getobjects.yaml +++ b/staging/src/k8s.io/kubectl/pkg/cmd/apply/testdata/prune/simple/manifest1-expected-getobjects.yaml @@ -2,7 +2,7 @@ apiVersion: v1 kind: Namespace metadata: labels: - applyset.k8s.io/part-of: placeholder-todo + applyset.k8s.io/part-of: applyset-bjd1LnyQq0mtUu-riZCqjDQOmh0iNb9O2RcuT12WR0k-v1 name: foo --- @@ -11,5 +11,5 @@ apiVersion: v1 kind: Namespace metadata: labels: - applyset.k8s.io/part-of: placeholder-todo + applyset.k8s.io/part-of: applyset-bjd1LnyQq0mtUu-riZCqjDQOmh0iNb9O2RcuT12WR0k-v1 name: bar diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/apply/testdata/prune/simple/manifest2-expected-getobjects.yaml b/staging/src/k8s.io/kubectl/pkg/cmd/apply/testdata/prune/simple/manifest2-expected-getobjects.yaml index 0e071e8cce0..552981acd5c 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/apply/testdata/prune/simple/manifest2-expected-getobjects.yaml +++ b/staging/src/k8s.io/kubectl/pkg/cmd/apply/testdata/prune/simple/manifest2-expected-getobjects.yaml @@ -2,5 +2,5 @@ apiVersion: v1 kind: Namespace metadata: labels: - applyset.k8s.io/part-of: placeholder-todo + applyset.k8s.io/part-of: applyset-bjd1LnyQq0mtUu-riZCqjDQOmh0iNb9O2RcuT12WR0k-v1 name: foo