diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/webhook_converter_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/webhook_converter_test.go index 01c97e90b58..50e337dc352 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/webhook_converter_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/webhook_converter_test.go @@ -28,11 +28,11 @@ import ( func TestRestoreObjectMeta(t *testing.T) { tests := []struct { - name string - original map[string]interface{} - converted map[string]interface{} - expected map[string]interface{} - wantErr bool + name string + original map[string]interface{} + converted map[string]interface{} + expected map[string]interface{} + expectedError bool }{ {"no converted metadata", map[string]interface{}{"metadata": map[string]interface{}{}, "spec": map[string]interface{}{}}, @@ -187,9 +187,9 @@ func TestRestoreObjectMeta(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if err := restoreObjectMeta(&unstructured.Unstructured{Object: tt.original}, &unstructured.Unstructured{Object: tt.converted}); err == nil && tt.wantErr { + if err := restoreObjectMeta(&unstructured.Unstructured{Object: tt.original}, &unstructured.Unstructured{Object: tt.converted}); err == nil && tt.expectedError { t.Fatalf("expected error, but didn't get one") - } else if err != nil && !tt.wantErr { + } else if err != nil && !tt.expectedError { t.Fatalf("unexpected error: %v", err) } diff --git a/staging/src/k8s.io/apiextensions-apiserver/test/integration/conversion/conversion_test.go b/staging/src/k8s.io/apiextensions-apiserver/test/integration/conversion/conversion_test.go index 192f0b9aada..e0c77cb8b06 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/test/integration/conversion/conversion_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/test/integration/conversion/conversion_test.go @@ -82,6 +82,16 @@ func testWebhookConverter(t *testing.T, pruning bool) { handler: NewObjectConverterWebhookHandler(t, nontrivialConverter), checks: checks(validateStorageVersion, validateServed, validateMixedStorageVersions("v1alpha1", "v1beta1", "v1beta2"), validateNonTrivialConverted, validateNonTrivialConvertedList, validateStoragePruning), }, + { + group: "metadata-mutating-converter", + handler: NewObjectConverterWebhookHandler(t, metadataMutatingConverter), + checks: checks(validateObjectMetaMutation), + }, + { + group: "metadata-uid-mutating-converter", + handler: NewObjectConverterWebhookHandler(t, uidMutatingConverter), + checks: checks(validateUIDMutation), + }, { group: "empty-response", handler: NewReviewWebhookHandler(t, emptyResponseConverter), @@ -408,6 +418,108 @@ func validateStoragePruning(t *testing.T, ctc *conversionTestContext) { } } +func validateObjectMetaMutation(t *testing.T, ctc *conversionTestContext) { + ns := ctc.namespace + + t.Logf("Creating object in storage version v1beta1") + storageVersion := "v1beta1" + ctc.setAndWaitStorageVersion(t, storageVersion) + name := "objectmeta-mutation-" + storageVersion + client := ctc.versionedClient(ns, storageVersion) + obj, err := client.Create(newConversionMultiVersionFixture(ns, name, storageVersion), metav1.CreateOptions{}) + if err != nil { + t.Fatal(err) + } + validateObjectMetaMutationObject(t, false, false, obj) + + t.Logf("Getting object in other version v1beta2") + client = ctc.versionedClient(ns, "v1beta2") + obj, err = client.Get(name, metav1.GetOptions{}) + if err != nil { + t.Fatal(err) + } + validateObjectMetaMutationObject(t, true, true, obj) + + t.Logf("Creating object in non-storage version") + name = "objectmeta-mutation-v1beta2" + client = ctc.versionedClient(ns, "v1beta2") + obj, err = client.Create(newConversionMultiVersionFixture(ns, name, "v1beta2"), metav1.CreateOptions{}) + if err != nil { + t.Fatal(err) + } + validateObjectMetaMutationObject(t, true, true, obj) + + t.Logf("Listing objects in non-storage version") + client = ctc.versionedClient(ns, "v1beta2") + list, err := client.List(metav1.ListOptions{}) + if err != nil { + t.Fatal(err) + } + for _, obj := range list.Items { + validateObjectMetaMutationObject(t, true, true, &obj) + } +} + +func validateObjectMetaMutationObject(t *testing.T, expectAnnotations, expectLabels bool, obj *unstructured.Unstructured) { + if expectAnnotations { + if _, found := obj.GetAnnotations()["from"]; !found { + t.Errorf("expected 'from=stable.example.com/v1beta1' annotation") + } + if _, found := obj.GetAnnotations()["to"]; !found { + t.Errorf("expected 'to=stable.example.com/v1beta2' annotation") + } + } else { + if v, found := obj.GetAnnotations()["from"]; found { + t.Errorf("unexpected 'from' annotation: %s", v) + } + if v, found := obj.GetAnnotations()["to"]; found { + t.Errorf("unexpected 'to' annotation: %s", v) + } + } + if expectLabels { + if _, found := obj.GetLabels()["from"]; !found { + t.Errorf("expected 'from=stable.example.com.v1beta1' label") + } + if _, found := obj.GetLabels()["to"]; !found { + t.Errorf("expected 'to=stable.example.com.v1beta2' label") + } + } else { + if v, found := obj.GetLabels()["from"]; found { + t.Errorf("unexpected 'from' label: %s", v) + } + if v, found := obj.GetLabels()["to"]; found { + t.Errorf("unexpected 'to' label: %s", v) + } + } + if sets.NewString(obj.GetFinalizers()...).Has("foo") { + t.Errorf("unexpected 'foo' finalizer") + } + if obj.GetGeneration() == 42 { + t.Errorf("unexpected generation 42") + } + if v, found, err := unstructured.NestedString(obj.Object, "metadata", "garbage"); err != nil { + t.Errorf("unexpected error accessing 'metadata.garbage': %v", err) + } else if found { + t.Errorf("unexpected 'metadata.garbage': %s", v) + } +} + +func validateUIDMutation(t *testing.T, ctc *conversionTestContext) { + ns := ctc.namespace + + t.Logf("Creating object in non-storage version v1beta1") + storageVersion := "v1beta1" + ctc.setAndWaitStorageVersion(t, storageVersion) + name := "uid-mutation-" + storageVersion + client := ctc.versionedClient(ns, "v1beta2") + obj, err := client.Create(newConversionMultiVersionFixture(ns, name, "v1beta2"), metav1.CreateOptions{}) + if err == nil { + t.Fatalf("expected creation error, but got: %v", obj) + } else if !strings.Contains(err.Error(), "must have the same UID") { + t.Errorf("expected 'must have the same UID' error message, but got: %v", err) + } +} + func expectConversionFailureMessage(id, message string) func(t *testing.T, ctc *conversionTestContext) { return func(t *testing.T, ctc *conversionTestContext) { ns := ctc.namespace @@ -533,6 +645,87 @@ func nontrivialConverter(desiredAPIVersion string, obj runtime.RawExtension) (ru return runtime.RawExtension{Raw: raw}, nil } +func metadataMutatingConverter(desiredAPIVersion string, obj runtime.RawExtension) (runtime.RawExtension, error) { + obj, err := nontrivialConverter(desiredAPIVersion, obj) + if err != nil { + return runtime.RawExtension{}, err + } + + u := &unstructured.Unstructured{Object: map[string]interface{}{}} + if err := json.Unmarshal(obj.Raw, u); err != nil { + return runtime.RawExtension{}, fmt.Errorf("failed to deserialize object: %s with error: %v", string(obj.Raw), err) + } + + // do not mutate the marker or the probe objects + if !strings.Contains(u.GetName(), "mutation") { + return obj, nil + } + + currentAPIVersion := u.GetAPIVersion() + + // mutate annotations. This should be persisted. + annotations := u.GetAnnotations() + if annotations == nil { + annotations = map[string]string{} + } + annotations["from"] = currentAPIVersion + annotations["to"] = desiredAPIVersion + u.SetAnnotations(annotations) + + // mutate labels. This should be persisted. + labels := u.GetLabels() + if labels == nil { + labels = map[string]string{} + } + labels["from"] = strings.Replace(currentAPIVersion, "/", ".", 1) // replace / with . because label values do not allow / + labels["to"] = strings.Replace(desiredAPIVersion, "/", ".", 1) + u.SetLabels(labels) + + // mutate other fields. This should be ignored. + u.SetGeneration(42) + u.SetOwnerReferences([]metav1.OwnerReference{{ + APIVersion: "v1", + Kind: "Namespace", + Name: "default", + UID: "1234", + Controller: nil, + BlockOwnerDeletion: nil, + }}) + u.SetResourceVersion("42") + u.SetFinalizers([]string{"foo"}) + if err := unstructured.SetNestedField(u.Object, "foo", "metadata", "garbage"); err != nil { + return runtime.RawExtension{}, err + } + + raw, err := json.Marshal(u) + if err != nil { + return runtime.RawExtension{}, fmt.Errorf("failed to serialize object: %v with error: %v", u, err) + } + return runtime.RawExtension{Raw: raw}, nil +} + +func uidMutatingConverter(desiredAPIVersion string, obj runtime.RawExtension) (runtime.RawExtension, error) { + u := &unstructured.Unstructured{Object: map[string]interface{}{}} + if err := json.Unmarshal(obj.Raw, u); err != nil { + return runtime.RawExtension{}, fmt.Errorf("failed to deserialize object: %s with error: %v", string(obj.Raw), err) + } + + // do not mutate the marker or the probe objects + if strings.Contains(u.GetName(), "mutation") { + // mutate other fields. This should be ignored. + if err := unstructured.SetNestedField(u.Object, "42", "metadata", "uid"); err != nil { + return runtime.RawExtension{}, err + } + } + + u.Object["apiVersion"] = desiredAPIVersion + raw, err := json.Marshal(u) + if err != nil { + return runtime.RawExtension{}, fmt.Errorf("failed to serialize object: %v with error: %v", u, err) + } + return runtime.RawExtension{Raw: raw}, nil +} + func newConversionTestContext(t *testing.T, apiExtensionsClient clientset.Interface, dynamicClient dynamic.Interface, etcdObjectReader *storage.EtcdObjectReader, crd *apiextensionsv1beta1.CustomResourceDefinition) (func(), *conversionTestContext) { crd, err := fixtures.CreateNewCustomResourceDefinition(crd, apiExtensionsClient, dynamicClient) if err != nil {