From 97b48eb5ecebd561034693d841d7f4587c02c15e Mon Sep 17 00:00:00 2001 From: Binbin Li Date: Thu, 21 Sep 2023 07:04:52 +0000 Subject: [PATCH] fix: fix broken test --- Makefile | 2 +- api/unversioned/policy_types.go | 2 - api/v1alpha1/policy_conversion.go | 14 +++- api/v1alpha1/policy_conversion_test.go | 83 ++++++++++++++++++++++ api/v1beta1/policy_types.go | 2 - internal/constants/constants.go | 19 +++++ pkg/controllers/policy_controller.go | 5 +- pkg/policyprovider/factory/factory_test.go | 2 +- 8 files changed, 119 insertions(+), 10 deletions(-) create mode 100644 api/v1alpha1/policy_conversion_test.go create mode 100644 internal/constants/constants.go diff --git a/Makefile b/Makefile index c0683b62b..ce35ff9f7 100644 --- a/Makefile +++ b/Makefile @@ -135,7 +135,7 @@ delete-gatekeeper: .PHONY: test-e2e test-e2e: generate-rotation-certs - bats -t ${BATS_BASE_TESTS_FILE} + timeout 15m bats -t ${BATS_BASE_TESTS_FILE} EXPIRING_CERT_DIR=.staging/rotation/expiring-certs CERT_DIR=.staging/rotation GATEKEEPER_VERSION=${GATEKEEPER_VERSION} bats -t ${BATS_PLUGIN_TESTS_FILE} .PHONY: test-e2e-cli diff --git a/api/unversioned/policy_types.go b/api/unversioned/policy_types.go index 1aaead7c5..345d4aff9 100644 --- a/api/unversioned/policy_types.go +++ b/api/unversioned/policy_types.go @@ -27,7 +27,6 @@ type PolicySpec struct { // Type of the policy Type string `json:"type,omitempty"` - // Parameters for this policy Parameters runtime.RawExtension `json:"parameters,omitempty"` } @@ -39,7 +38,6 @@ type PolicyStatus struct { // Is successful while applying the policy. IsSuccess bool `json:"issuccess"` - // Error message if policy is not successfully applied. // +optional Error string `json:"error,omitempty"` diff --git a/api/v1alpha1/policy_conversion.go b/api/v1alpha1/policy_conversion.go index 7aeb061a9..b67653eb4 100644 --- a/api/v1alpha1/policy_conversion.go +++ b/api/v1alpha1/policy_conversion.go @@ -18,23 +18,31 @@ package v1alpha1 import ( unversioned "github.com/deislabs/ratify/api/unversioned" + "github.com/deislabs/ratify/internal/constants" conversion "k8s.io/apimachinery/pkg/conversion" ) +// Convert unversioned PolicySpec to PolicySpec of v1alpha1. +// //nolint:revive // ignore linter for autogenerated code func Convert_unversioned_PolicySpec_To_v1alpha1_PolicySpec(in *unversioned.PolicySpec, out *PolicySpec, _ conversion.Scope) error { out.Parameters = in.Parameters return nil } +// Convert unversioned PolicyStatus to PolicyStatus of v1alpha1. +// //nolint:revive // ignore linter for autogenerated code func Convert_unversioned_PolicyStatus_To_v1alpha1_PolicyStatus(in *unversioned.PolicyStatus, out *PolicyStatus, _ conversion.Scope) error { return nil } +// Convert unversioned Policy to Policy of v1alpha1. +// //nolint:revive // ignore linter for autogenerated code func Convert_unversioned_Policy_To_v1alpha1_Policy(in *unversioned.Policy, out *Policy, s conversion.Scope) error { out.ObjectMeta = in.ObjectMeta + // metadata.name in v1alpha1 is same as spec.type in unversioned. out.ObjectMeta.Name = in.Spec.Type if err := Convert_unversioned_PolicySpec_To_v1alpha1_PolicySpec(&in.Spec, &out.Spec, s); err != nil { return err @@ -42,13 +50,17 @@ func Convert_unversioned_Policy_To_v1alpha1_Policy(in *unversioned.Policy, out * return Convert_unversioned_PolicyStatus_To_v1alpha1_PolicyStatus(&in.Status, &out.Status, s) } +// Convert Policy of v1alpha1 to unversioned Policy. +// //nolint:revive // ignore linter for autogenerated code func Convert_v1alpha1_Policy_To_unversioned_Policy(in *Policy, out *unversioned.Policy, s conversion.Scope) error { out.ObjectMeta = in.ObjectMeta - out.ObjectMeta.Name = "ratify-policy" + // metadata.name MUST be `ratify-policy` in unversioned. + out.ObjectMeta.Name = constants.RatifyPolicy if err := Convert_v1alpha1_PolicySpec_To_unversioned_PolicySpec(&in.Spec, &out.Spec, s); err != nil { return err } + // spec.type in unversioned is same as metadata.name in v1alpha1. out.Spec.Type = in.ObjectMeta.Name return Convert_v1alpha1_PolicyStatus_To_unversioned_PolicyStatus(&in.Status, &out.Status, s) } diff --git a/api/v1alpha1/policy_conversion_test.go b/api/v1alpha1/policy_conversion_test.go new file mode 100644 index 000000000..862e16c63 --- /dev/null +++ b/api/v1alpha1/policy_conversion_test.go @@ -0,0 +1,83 @@ +/* +Copyright The Ratify Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1alpha1 + +import ( + "reflect" + "testing" + + unversioned "github.com/deislabs/ratify/api/unversioned" + "github.com/deislabs/ratify/internal/constants" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + runtime "k8s.io/apimachinery/pkg/runtime" +) + +var params = runtime.RawExtension{} + +const testPolicyType = "testPolicyType" + +func TestConvert_unversioned_PolicySpec_To_v1alpha1_PolicySpec(t *testing.T) { + in := &unversioned.PolicySpec{ + Parameters: params, + } + out := &PolicySpec{} + if err := Convert_unversioned_PolicySpec_To_v1alpha1_PolicySpec(in, out, nil); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !reflect.DeepEqual(out.Parameters, in.Parameters) { + t.Fatalf("expect parameters to be equal, but got different values") + } +} + +func TestConvert_unversioned_PolicyStatus_To_v1alpha1_PolicyStatus(t *testing.T) { + if err := Convert_unversioned_PolicyStatus_To_v1alpha1_PolicyStatus(nil, nil, nil); err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestConvert_unversioned_Policy_To_v1alpha1_Policy(t *testing.T) { + in := &unversioned.Policy{ + Spec: unversioned.PolicySpec{ + Type: testPolicyType, + }, + } + out := &Policy{} + if err := Convert_unversioned_Policy_To_v1alpha1_Policy(in, out, nil); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if out.ObjectMeta.Name != in.Spec.Type { + t.Fatalf("expect metadata.name to be %s, but got %s", in.Spec.Type, out.ObjectMeta.Name) + } +} + +func TestConvert_v1alpha1_Policy_To_unversioned_Policy(t *testing.T) { + in := &Policy{ + ObjectMeta: metav1.ObjectMeta{ + Name: testPolicyType, + }, + } + out := &unversioned.Policy{} + if err := Convert_v1alpha1_Policy_To_unversioned_Policy(in, out, nil); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if out.ObjectMeta.Name != constants.RatifyPolicy { + t.Fatalf("expect metadata.name to be %s, but got %s", constants.RatifyPolicy, out.ObjectMeta.Name) + } + if out.Spec.Type != in.ObjectMeta.Name { + t.Fatalf("expect spec.type to be %s, but got %s", in.ObjectMeta.Name, out.Spec.Type) + } +} diff --git a/api/v1beta1/policy_types.go b/api/v1beta1/policy_types.go index 48909f454..c64d4315b 100644 --- a/api/v1beta1/policy_types.go +++ b/api/v1beta1/policy_types.go @@ -30,7 +30,6 @@ type PolicySpec struct { // Type of the policy Type string `json:"type,omitempty"` - // +kubebuilder:pruning:PreserveUnknownFields // Parameters for this policy Parameters runtime.RawExtension `json:"parameters,omitempty"` @@ -42,7 +41,6 @@ type PolicyStatus struct { // Is successful while applying the policy. IsSuccess bool `json:"issuccess"` - // Error message if policy is not successfully applied. // +optional Error string `json:"error,omitempty"` diff --git a/internal/constants/constants.go b/internal/constants/constants.go new file mode 100644 index 000000000..104646f4b --- /dev/null +++ b/internal/constants/constants.go @@ -0,0 +1,19 @@ +/* +Copyright The Ratify Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package constants + +const RatifyPolicy = "ratify-policy" diff --git a/pkg/controllers/policy_controller.go b/pkg/controllers/policy_controller.go index 2fe678445..109071c40 100644 --- a/pkg/controllers/policy_controller.go +++ b/pkg/controllers/policy_controller.go @@ -21,6 +21,7 @@ import ( "fmt" configv1beta1 "github.com/deislabs/ratify/api/v1beta1" + "github.com/deislabs/ratify/internal/constants" "github.com/deislabs/ratify/pkg/policyprovider" "github.com/deislabs/ratify/pkg/policyprovider/config" pf "github.com/deislabs/ratify/pkg/policyprovider/factory" @@ -31,8 +32,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) -const ratifyPolicy = "ratify-policy" - // PolicyReconciler reconciles a Policy object type PolicyReconciler struct { client.Client @@ -76,7 +75,7 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr return ctrl.Result{}, client.IgnoreNotFound(err) } - if resource != ratifyPolicy { + if resource != constants.RatifyPolicy { errStr := fmt.Sprintf("metadata.name must be ratify-policy, got %s", resource) policyLogger.Error(errStr) writePolicyStatus(ctx, r, &policy, policyLogger, false, errStr) diff --git a/pkg/policyprovider/factory/factory_test.go b/pkg/policyprovider/factory/factory_test.go index a0171049e..157c6972e 100644 --- a/pkg/policyprovider/factory/factory_test.go +++ b/pkg/policyprovider/factory/factory_test.go @@ -32,7 +32,7 @@ func (f *TestPolicyProviderFactory) Create(_ config.PolicyPluginConfig) (policyp // Checks the correct registered policy provider is invoked based on config func TestCreatePolicyProvidersFromConfig_BuiltInPolicyProviders_ReturnsExpected(t *testing.T) { builtInPolicyProviders = map[string]PolicyFactory{ - "test-policyprovider": &TestPolicyProviderFactory{}, + "testpolicyprovider": &TestPolicyProviderFactory{}, } configPolicyConfig := map[string]interface{}{