diff --git a/pkg/controller/certmanager/cert_manager_networkpolicy_test.go b/pkg/controller/certmanager/cert_manager_networkpolicy_test.go new file mode 100644 index 000000000..92c238d1b --- /dev/null +++ b/pkg/controller/certmanager/cert_manager_networkpolicy_test.go @@ -0,0 +1,308 @@ +package certmanager + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/openshift/cert-manager-operator/api/operator/v1alpha1" +) + +func TestValidateComponentName(t *testing.T) { + tests := []struct { + name string + componentName v1alpha1.ComponentName + expectError bool + }{ + { + name: "CoreController is valid", + componentName: v1alpha1.CoreController, + expectError: false, + }, + { + name: "CAInjector is valid", + componentName: v1alpha1.CAInjector, + expectError: false, + }, + { + name: "Webhook is valid", + componentName: v1alpha1.Webhook, + expectError: false, + }, + { + name: "empty string is invalid", + componentName: "", + expectError: true, + }, + { + name: "unknown component name is invalid", + componentName: "UnknownComponent", + expectError: true, + }, + { + name: "lowercase controller is invalid", + componentName: "controller", + expectError: true, + }, + } + + c := &CertManagerNetworkPolicyUserDefinedController{} + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + err := c.validateComponentName(tc.componentName) + if tc.expectError { + assert.Error(t, err) + assert.Contains(t, err.Error(), "unsupported component name") + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestGetPodSelectorForComponent(t *testing.T) { + tests := []struct { + name string + componentName v1alpha1.ComponentName + expectedLabels map[string]string + }{ + { + name: "CoreController returns cert-manager app label", + componentName: v1alpha1.CoreController, + expectedLabels: map[string]string{ + "app": "cert-manager", + }, + }, + { + name: "CAInjector returns cainjector app label", + componentName: v1alpha1.CAInjector, + expectedLabels: map[string]string{ + "app": "cainjector", + }, + }, + { + name: "Webhook returns webhook app label", + componentName: v1alpha1.Webhook, + expectedLabels: map[string]string{ + "app": "webhook", + }, + }, + { + name: "unknown component returns default label", + componentName: "Unknown", + expectedLabels: map[string]string{ + "app.kubernetes.io/name": "cert-manager", + }, + }, + } + + c := &CertManagerNetworkPolicyUserDefinedController{} + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + selector := c.getPodSelectorForComponent(tc.componentName) + require.Equal(t, tc.expectedLabels, selector.MatchLabels) + }) + } +} + +func TestValidateNetworkPolicyConfig(t *testing.T) { + tests := []struct { + name string + certManager *v1alpha1.CertManager + expectError bool + errContains string + }{ + { + name: "valid config with single policy passes", + certManager: &v1alpha1.CertManager{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: v1alpha1.CertManagerSpec{ + NetworkPolicies: []v1alpha1.NetworkPolicy{ + { + Name: "allow-egress", + ComponentName: v1alpha1.CoreController, + }, + }, + }, + }, + expectError: false, + }, + { + name: "valid config with multiple policies passes", + certManager: &v1alpha1.CertManager{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: v1alpha1.CertManagerSpec{ + NetworkPolicies: []v1alpha1.NetworkPolicy{ + { + Name: "allow-egress-controller", + ComponentName: v1alpha1.CoreController, + }, + { + Name: "allow-egress-webhook", + ComponentName: v1alpha1.Webhook, + }, + { + Name: "allow-egress-cainjector", + ComponentName: v1alpha1.CAInjector, + }, + }, + }, + }, + expectError: false, + }, + { + name: "empty network policies list passes", + certManager: &v1alpha1.CertManager{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: v1alpha1.CertManagerSpec{ + NetworkPolicies: []v1alpha1.NetworkPolicy{}, + }, + }, + expectError: false, + }, + { + name: "nil network policies list passes", + certManager: &v1alpha1.CertManager{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: v1alpha1.CertManagerSpec{}, + }, + expectError: false, + }, + { + name: "invalid component name fails", + certManager: &v1alpha1.CertManager{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: v1alpha1.CertManagerSpec{ + NetworkPolicies: []v1alpha1.NetworkPolicy{ + { + Name: "bad-policy", + ComponentName: "InvalidComponent", + }, + }, + }, + }, + expectError: true, + errContains: "invalid component name", + }, + { + name: "empty policy name fails", + certManager: &v1alpha1.CertManager{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: v1alpha1.CertManagerSpec{ + NetworkPolicies: []v1alpha1.NetworkPolicy{ + { + Name: "", + ComponentName: v1alpha1.CoreController, + }, + }, + }, + }, + expectError: true, + errContains: "name cannot be empty", + }, + { + name: "second policy with invalid component fails", + certManager: &v1alpha1.CertManager{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: v1alpha1.CertManagerSpec{ + NetworkPolicies: []v1alpha1.NetworkPolicy{ + { + Name: "good-policy", + ComponentName: v1alpha1.CoreController, + }, + { + Name: "bad-policy", + ComponentName: "BadComponent", + }, + }, + }, + }, + expectError: true, + errContains: "network policy at index 1", + }, + } + + c := &CertManagerNetworkPolicyUserDefinedController{} + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + err := c.validateNetworkPolicyConfig(tc.certManager) + if tc.expectError { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.errContains) + } else { + require.NoError(t, err) + } + }) + } +} + +func TestCreateUserNetworkPolicy(t *testing.T) { + tests := []struct { + name string + userPolicy v1alpha1.NetworkPolicy + expectedName string + expectedLabels map[string]string + expectedPodLabels map[string]string + }{ + { + name: "creates policy for CoreController", + userPolicy: v1alpha1.NetworkPolicy{ + Name: "allow-dns", + ComponentName: v1alpha1.CoreController, + }, + expectedName: "cert-manager-user-allow-dns", + expectedLabels: map[string]string{ + networkPolicyOwnerLabel: "cert-manager", + }, + expectedPodLabels: map[string]string{ + "app": "cert-manager", + }, + }, + { + name: "creates policy for Webhook", + userPolicy: v1alpha1.NetworkPolicy{ + Name: "allow-api", + ComponentName: v1alpha1.Webhook, + }, + expectedName: "cert-manager-user-allow-api", + expectedLabels: map[string]string{ + networkPolicyOwnerLabel: "cert-manager", + }, + expectedPodLabels: map[string]string{ + "app": "webhook", + }, + }, + { + name: "creates policy for CAInjector", + userPolicy: v1alpha1.NetworkPolicy{ + Name: "allow-egress", + ComponentName: v1alpha1.CAInjector, + }, + expectedName: "cert-manager-user-allow-egress", + expectedLabels: map[string]string{ + networkPolicyOwnerLabel: "cert-manager", + }, + expectedPodLabels: map[string]string{ + "app": "cainjector", + }, + }, + } + + c := &CertManagerNetworkPolicyUserDefinedController{} + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + policy := c.createUserNetworkPolicy(tc.userPolicy) + + require.Equal(t, tc.expectedName, policy.Name) + require.Equal(t, certManagerNamespace, policy.Namespace) + require.Equal(t, tc.expectedLabels, policy.Labels) + require.Equal(t, tc.expectedPodLabels, policy.Spec.PodSelector.MatchLabels) + }) + } +} diff --git a/pkg/controller/certmanager/default_cert_manager_controller_test.go b/pkg/controller/certmanager/default_cert_manager_controller_test.go new file mode 100644 index 000000000..a9113fee2 --- /dev/null +++ b/pkg/controller/certmanager/default_cert_manager_controller_test.go @@ -0,0 +1,58 @@ +package certmanager + +import ( + "context" + "testing" + + operatorv1 "github.com/openshift/api/operator/v1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/openshift/cert-manager-operator/api/operator/v1alpha1" + fakeclientset "github.com/openshift/cert-manager-operator/pkg/operator/clientset/versioned/fake" +) + +func TestCreateDefaultCertManager(t *testing.T) { + fakeClient := fakeclientset.NewSimpleClientset() + + controller := &DefaultCertManagerController{ + certManagerClient: fakeClient.OperatorV1alpha1(), + } + + ctx := context.Background() + cm, err := controller.createDefaultCertManager(ctx) + require.NoError(t, err) + require.NotNil(t, cm) + + assert.Equal(t, "cluster", cm.Name) + assert.Equal(t, operatorv1.Managed, cm.Spec.ManagementState) + + // Verify the resource was actually created in the fake client + got, err := fakeClient.OperatorV1alpha1().CertManagers().Get(ctx, "cluster", metav1.GetOptions{}) + require.NoError(t, err) + assert.Equal(t, "cluster", got.Name) + assert.Equal(t, operatorv1.Managed, got.Spec.ManagementState) +} + +func TestCreateDefaultCertManagerAlreadyExists(t *testing.T) { + existing := &v1alpha1.CertManager{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster", + }, + Spec: v1alpha1.CertManagerSpec{ + OperatorSpec: operatorv1.OperatorSpec{ + ManagementState: operatorv1.Managed, + }, + }, + } + fakeClient := fakeclientset.NewSimpleClientset(existing) + + controller := &DefaultCertManagerController{ + certManagerClient: fakeClient.OperatorV1alpha1(), + } + + ctx := context.Background() + _, err := controller.createDefaultCertManager(ctx) + require.Error(t, err, "creating a duplicate CertManager should fail") +} diff --git a/pkg/controller/certmanager/deployment_helper_test.go b/pkg/controller/certmanager/deployment_helper_test.go index b48041d0a..eb587ae9b 100644 --- a/pkg/controller/certmanager/deployment_helper_test.go +++ b/pkg/controller/certmanager/deployment_helper_test.go @@ -878,6 +878,438 @@ func TestGetOverrideSchedulingFor(t *testing.T) { } } +func TestGetOverrideArgsFor(t *testing.T) { + tests := []struct { + name string + certManagerObj v1alpha1.CertManager + deploymentName string + expectedArgs []string + expectError bool + errContains string + }{ + { + name: "get override args for controller", + certManagerObj: v1alpha1.CertManager{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: v1alpha1.CertManagerSpec{ + ControllerConfig: &v1alpha1.DeploymentConfig{ + OverrideArgs: []string{"--v=4", "--feature-gates=ExperimentalGatewayAPISupport=true"}, + }, + }, + }, + deploymentName: certmanagerControllerDeployment, + expectedArgs: []string{"--v=4", "--feature-gates=ExperimentalGatewayAPISupport=true"}, + }, + { + name: "get override args for webhook", + certManagerObj: v1alpha1.CertManager{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: v1alpha1.CertManagerSpec{ + WebhookConfig: &v1alpha1.DeploymentConfig{ + OverrideArgs: []string{"--secure-port=10251"}, + }, + }, + }, + deploymentName: certmanagerWebhookDeployment, + expectedArgs: []string{"--secure-port=10251"}, + }, + { + name: "get override args for cainjector", + certManagerObj: v1alpha1.CertManager{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: v1alpha1.CertManagerSpec{ + CAInjectorConfig: &v1alpha1.DeploymentConfig{ + OverrideArgs: []string{"--leader-elect=false"}, + }, + }, + }, + deploymentName: certmanagerCAinjectorDeployment, + expectedArgs: []string{"--leader-elect=false"}, + }, + { + name: "nil config returns nil args for controller", + certManagerObj: v1alpha1.CertManager{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: v1alpha1.CertManagerSpec{}, + }, + deploymentName: certmanagerControllerDeployment, + expectedArgs: nil, + }, + { + name: "unsupported deployment name returns error", + certManagerObj: v1alpha1.CertManager{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: v1alpha1.CertManagerSpec{}, + }, + deploymentName: "unknown-deployment", + expectError: true, + errContains: "unsupported deployment name", + }, + } + + ctx := t.Context() + + watcherStarted := make(chan struct{}) + fakeClient := fake.NewSimpleClientset() + fakeClient.PrependWatchReactor("certmanagers", func(action clienttesting.Action) (handled bool, ret watch.Interface, err error) { + gvr := action.GetResource() + ns := action.GetNamespace() + watch, err := fakeClient.Tracker().Watch(gvr, ns) + if err != nil { + return false, nil, err + } + close(watcherStarted) + return true, watch, nil + }) + + certManagerInformers := certmanoperatorinformer.NewSharedInformerFactory(fakeClient, 0).Operator().V1alpha1().CertManagers() + certManagerChan := make(chan *v1alpha1.CertManager, 1) + + certManagerInformers.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ + AddFunc: func(obj any) { + certManagerChan <- obj.(*v1alpha1.CertManager) + }, + DeleteFunc: func(obj any) { + certManagerChan <- obj.(*v1alpha1.CertManager) + }, + }) + + go certManagerInformers.Informer().Run(ctx.Done()) + cache.WaitForCacheSync(ctx.Done(), certManagerInformers.Informer().HasSynced) + <-watcherStarted + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + _, err := fakeClient.OperatorV1alpha1().CertManagers().Create(ctx, &tc.certManagerObj, metav1.CreateOptions{}) + if err != nil { + t.Fatalf("error injecting cert manager add: %v", err) + } + + select { + case <-certManagerChan: + case <-time.After(wait.ForeverTestTimeout): + t.Fatal("Informer did not get the added cert manager object") + } + + actualArgs, err := getOverrideArgsFor(certManagerInformers, tc.deploymentName) + if tc.expectError { + assert.Error(t, err) + assert.Contains(t, err.Error(), tc.errContains) + } else { + assert.NoError(t, err) + require.Equal(t, tc.expectedArgs, actualArgs) + } + + err = fakeClient.OperatorV1alpha1().CertManagers().Delete(ctx, tc.certManagerObj.Name, metav1.DeleteOptions{}) + if err != nil { + t.Fatalf("error deleting cert manager: %v", err) + } + + select { + case <-certManagerChan: + case <-time.After(wait.ForeverTestTimeout): + t.Fatal("Informer did not get the deleted cert manager") + } + }) + } +} + +func TestGetOverrideEnvFor(t *testing.T) { + tests := []struct { + name string + certManagerObj v1alpha1.CertManager + deploymentName string + expectedEnv []corev1.EnvVar + expectError bool + errContains string + }{ + { + name: "get override env for controller", + certManagerObj: v1alpha1.CertManager{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: v1alpha1.CertManagerSpec{ + ControllerConfig: &v1alpha1.DeploymentConfig{ + OverrideEnv: []corev1.EnvVar{ + {Name: "HTTP_PROXY", Value: "http://proxy:3128"}, + }, + }, + }, + }, + deploymentName: certmanagerControllerDeployment, + expectedEnv: []corev1.EnvVar{ + {Name: "HTTP_PROXY", Value: "http://proxy:3128"}, + }, + }, + { + name: "get override env for webhook", + certManagerObj: v1alpha1.CertManager{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: v1alpha1.CertManagerSpec{ + WebhookConfig: &v1alpha1.DeploymentConfig{ + OverrideEnv: []corev1.EnvVar{ + {Name: "MY_VAR", Value: "my-value"}, + }, + }, + }, + }, + deploymentName: certmanagerWebhookDeployment, + expectedEnv: []corev1.EnvVar{ + {Name: "MY_VAR", Value: "my-value"}, + }, + }, + { + name: "get override env for cainjector", + certManagerObj: v1alpha1.CertManager{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: v1alpha1.CertManagerSpec{ + CAInjectorConfig: &v1alpha1.DeploymentConfig{ + OverrideEnv: []corev1.EnvVar{ + {Name: "NO_PROXY", Value: "localhost"}, + }, + }, + }, + }, + deploymentName: certmanagerCAinjectorDeployment, + expectedEnv: []corev1.EnvVar{ + {Name: "NO_PROXY", Value: "localhost"}, + }, + }, + { + name: "nil config returns nil env", + certManagerObj: v1alpha1.CertManager{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: v1alpha1.CertManagerSpec{}, + }, + deploymentName: certmanagerControllerDeployment, + expectedEnv: nil, + }, + { + name: "unsupported deployment name returns error", + certManagerObj: v1alpha1.CertManager{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: v1alpha1.CertManagerSpec{}, + }, + deploymentName: "unknown-deployment", + expectError: true, + errContains: "unsupported deployment name", + }, + } + + ctx := t.Context() + + watcherStarted := make(chan struct{}) + fakeClient := fake.NewSimpleClientset() + fakeClient.PrependWatchReactor("certmanagers", func(action clienttesting.Action) (handled bool, ret watch.Interface, err error) { + gvr := action.GetResource() + ns := action.GetNamespace() + watch, err := fakeClient.Tracker().Watch(gvr, ns) + if err != nil { + return false, nil, err + } + close(watcherStarted) + return true, watch, nil + }) + + certManagerInformers := certmanoperatorinformer.NewSharedInformerFactory(fakeClient, 0).Operator().V1alpha1().CertManagers() + certManagerChan := make(chan *v1alpha1.CertManager, 1) + + certManagerInformers.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ + AddFunc: func(obj any) { + certManagerChan <- obj.(*v1alpha1.CertManager) + }, + DeleteFunc: func(obj any) { + certManagerChan <- obj.(*v1alpha1.CertManager) + }, + }) + + go certManagerInformers.Informer().Run(ctx.Done()) + cache.WaitForCacheSync(ctx.Done(), certManagerInformers.Informer().HasSynced) + <-watcherStarted + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + _, err := fakeClient.OperatorV1alpha1().CertManagers().Create(ctx, &tc.certManagerObj, metav1.CreateOptions{}) + if err != nil { + t.Fatalf("error injecting cert manager add: %v", err) + } + + select { + case <-certManagerChan: + case <-time.After(wait.ForeverTestTimeout): + t.Fatal("Informer did not get the added cert manager object") + } + + actualEnv, err := getOverrideEnvFor(certManagerInformers, tc.deploymentName) + if tc.expectError { + assert.Error(t, err) + assert.Contains(t, err.Error(), tc.errContains) + } else { + assert.NoError(t, err) + require.Equal(t, tc.expectedEnv, actualEnv) + } + + err = fakeClient.OperatorV1alpha1().CertManagers().Delete(ctx, tc.certManagerObj.Name, metav1.DeleteOptions{}) + if err != nil { + t.Fatalf("error deleting cert manager: %v", err) + } + + select { + case <-certManagerChan: + case <-time.After(wait.ForeverTestTimeout): + t.Fatal("Informer did not get the deleted cert manager") + } + }) + } +} + +func TestGetOverridePodLabelsFor(t *testing.T) { + tests := []struct { + name string + certManagerObj v1alpha1.CertManager + deploymentName string + expectedLabels map[string]string + expectError bool + errContains string + }{ + { + name: "get override labels for controller", + certManagerObj: v1alpha1.CertManager{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: v1alpha1.CertManagerSpec{ + ControllerConfig: &v1alpha1.DeploymentConfig{ + OverrideLabels: map[string]string{ + "custom-label": "custom-value", + }, + }, + }, + }, + deploymentName: certmanagerControllerDeployment, + expectedLabels: map[string]string{ + "custom-label": "custom-value", + }, + }, + { + name: "get override labels for webhook", + certManagerObj: v1alpha1.CertManager{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: v1alpha1.CertManagerSpec{ + WebhookConfig: &v1alpha1.DeploymentConfig{ + OverrideLabels: map[string]string{ + "env": "production", + }, + }, + }, + }, + deploymentName: certmanagerWebhookDeployment, + expectedLabels: map[string]string{ + "env": "production", + }, + }, + { + name: "get override labels for cainjector", + certManagerObj: v1alpha1.CertManager{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: v1alpha1.CertManagerSpec{ + CAInjectorConfig: &v1alpha1.DeploymentConfig{ + OverrideLabels: map[string]string{ + "team": "security", + }, + }, + }, + }, + deploymentName: certmanagerCAinjectorDeployment, + expectedLabels: map[string]string{ + "team": "security", + }, + }, + { + name: "nil config returns nil labels", + certManagerObj: v1alpha1.CertManager{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: v1alpha1.CertManagerSpec{}, + }, + deploymentName: certmanagerControllerDeployment, + expectedLabels: nil, + }, + { + name: "unsupported deployment name returns error", + certManagerObj: v1alpha1.CertManager{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: v1alpha1.CertManagerSpec{}, + }, + deploymentName: "unknown-deployment", + expectError: true, + errContains: "unsupported deployment name", + }, + } + + ctx := t.Context() + + watcherStarted := make(chan struct{}) + fakeClient := fake.NewSimpleClientset() + fakeClient.PrependWatchReactor("certmanagers", func(action clienttesting.Action) (handled bool, ret watch.Interface, err error) { + gvr := action.GetResource() + ns := action.GetNamespace() + watch, err := fakeClient.Tracker().Watch(gvr, ns) + if err != nil { + return false, nil, err + } + close(watcherStarted) + return true, watch, nil + }) + + certManagerInformers := certmanoperatorinformer.NewSharedInformerFactory(fakeClient, 0).Operator().V1alpha1().CertManagers() + certManagerChan := make(chan *v1alpha1.CertManager, 1) + + certManagerInformers.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ + AddFunc: func(obj any) { + certManagerChan <- obj.(*v1alpha1.CertManager) + }, + DeleteFunc: func(obj any) { + certManagerChan <- obj.(*v1alpha1.CertManager) + }, + }) + + go certManagerInformers.Informer().Run(ctx.Done()) + cache.WaitForCacheSync(ctx.Done(), certManagerInformers.Informer().HasSynced) + <-watcherStarted + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + _, err := fakeClient.OperatorV1alpha1().CertManagers().Create(ctx, &tc.certManagerObj, metav1.CreateOptions{}) + if err != nil { + t.Fatalf("error injecting cert manager add: %v", err) + } + + select { + case <-certManagerChan: + case <-time.After(wait.ForeverTestTimeout): + t.Fatal("Informer did not get the added cert manager object") + } + + actualLabels, err := getOverridePodLabelsFor(certManagerInformers, tc.deploymentName) + if tc.expectError { + assert.Error(t, err) + assert.Contains(t, err.Error(), tc.errContains) + } else { + assert.NoError(t, err) + require.Equal(t, tc.expectedLabels, actualLabels) + } + + err = fakeClient.OperatorV1alpha1().CertManagers().Delete(ctx, tc.certManagerObj.Name, metav1.DeleteOptions{}) + if err != nil { + t.Fatalf("error deleting cert manager: %v", err) + } + + select { + case <-certManagerChan: + case <-time.After(wait.ForeverTestTimeout): + t.Fatal("Informer did not get the deleted cert manager") + } + }) + } +} + func TestGetOverrideReplicasFor(t *testing.T) { tests := []struct { name string diff --git a/pkg/controller/certmanager/deployment_log_level_test.go b/pkg/controller/certmanager/deployment_log_level_test.go new file mode 100644 index 000000000..66a4a919d --- /dev/null +++ b/pkg/controller/certmanager/deployment_log_level_test.go @@ -0,0 +1,118 @@ +package certmanager + +import ( + "testing" + + operatorv1 "github.com/openshift/api/operator/v1" + "github.com/stretchr/testify/require" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func newTestDeploymentWithArgs(args []string) *appsv1.Deployment { + return &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cert-manager", + }, + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "cert-manager-controller", + Args: args, + }, + }, + }, + }, + }, + } +} + +func TestWithLogLevel(t *testing.T) { + tests := []struct { + name string + logLevel operatorv1.LogLevel + existingArgs []string + wantArgs []string + wantChanged bool + }{ + { + name: "Normal log level sets --v=2", + logLevel: operatorv1.Normal, + existingArgs: []string{"--cluster-resource-namespace=cert-manager"}, + wantArgs: []string{"--cluster-resource-namespace=cert-manager", "--v=2"}, + wantChanged: true, + }, + { + name: "Debug log level sets --v=4", + logLevel: operatorv1.Debug, + existingArgs: []string{"--cluster-resource-namespace=cert-manager"}, + wantArgs: []string{"--cluster-resource-namespace=cert-manager", "--v=4"}, + wantChanged: true, + }, + { + name: "Trace log level sets --v=6", + logLevel: operatorv1.Trace, + existingArgs: []string{"--cluster-resource-namespace=cert-manager"}, + wantArgs: []string{"--cluster-resource-namespace=cert-manager", "--v=6"}, + wantChanged: true, + }, + { + name: "TraceAll log level sets --v=8", + logLevel: operatorv1.TraceAll, + existingArgs: []string{"--cluster-resource-namespace=cert-manager"}, + wantArgs: []string{"--cluster-resource-namespace=cert-manager", "--v=8"}, + wantChanged: true, + }, + { + name: "empty log level does not modify args", + logLevel: "", + existingArgs: []string{"--cluster-resource-namespace=cert-manager", "--v=2"}, + wantArgs: []string{"--cluster-resource-namespace=cert-manager", "--v=2"}, + wantChanged: false, + }, + { + name: "unknown log level does not modify args", + logLevel: "UnknownLevel", + existingArgs: []string{"--cluster-resource-namespace=cert-manager", "--v=2"}, + wantArgs: []string{"--cluster-resource-namespace=cert-manager", "--v=2"}, + wantChanged: false, + }, + { + name: "log level overrides existing --v arg", + logLevel: operatorv1.Debug, + existingArgs: []string{"--cluster-resource-namespace=cert-manager", "--v=2"}, + wantArgs: []string{"--cluster-resource-namespace=cert-manager", "--v=4"}, + wantChanged: true, + }, + { + name: "log level merges with multiple existing args", + logLevel: operatorv1.Trace, + existingArgs: []string{"--leader-election-namespace=kube-system", "--v=2", "--cluster-resource-namespace=cert-manager"}, + wantArgs: []string{"--cluster-resource-namespace=cert-manager", "--leader-election-namespace=kube-system", "--v=6"}, + wantChanged: true, + }, + { + name: "log level works with empty existing args", + logLevel: operatorv1.Normal, + existingArgs: nil, + wantArgs: []string{"--v=2"}, + wantChanged: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + deployment := newTestDeploymentWithArgs(tc.existingArgs) + operatorSpec := &operatorv1.OperatorSpec{ + LogLevel: tc.logLevel, + } + + err := withLogLevel(operatorSpec, deployment) + require.NoError(t, err) + require.Equal(t, tc.wantArgs, deployment.Spec.Template.Spec.Containers[0].Args) + }) + } +} diff --git a/pkg/controller/certmanager/deployment_overrides_test.go b/pkg/controller/certmanager/deployment_overrides_test.go index e51fc669f..e1254ab3f 100644 --- a/pkg/controller/certmanager/deployment_overrides_test.go +++ b/pkg/controller/certmanager/deployment_overrides_test.go @@ -4,12 +4,14 @@ import ( "strings" "testing" + operatorv1 "github.com/openshift/api/operator/v1" "github.com/openshift/library-go/pkg/operator/resource/resourceread" "github.com/stretchr/testify/require" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" corelistersv1 "k8s.io/client-go/listers/core/v1" "k8s.io/client-go/tools/cache" @@ -226,6 +228,27 @@ func TestUnsupportedConfigOverrides(t *testing.T) { } } +func TestWithUnsupportedArgsOverrideHookInvalidJSON(t *testing.T) { + deployment := newTestDeploymentWithArgs([]string{"--v=2"}) + operatorSpec := &operatorv1.OperatorSpec{ + UnsupportedConfigOverrides: runtime.RawExtension{ + Raw: []byte(`{invalid-json`), + }, + } + + err := withUnsupportedArgsOverrideHook(operatorSpec, deployment) + require.Error(t, err, "expected error for invalid JSON in UnsupportedConfigOverrides") +} + +func TestWithUnsupportedArgsOverrideHookEmptyRaw(t *testing.T) { + deployment := newTestDeploymentWithArgs([]string{"--v=2"}) + operatorSpec := &operatorv1.OperatorSpec{} + + err := withUnsupportedArgsOverrideHook(operatorSpec, deployment) + require.NoError(t, err) + require.Equal(t, []string{"--v=2"}, deployment.Spec.Template.Spec.Containers[0].Args) +} + func TestParseEnvMap(t *testing.T) { tests := []struct { name string diff --git a/pkg/controller/certmanager/related_images_test.go b/pkg/controller/certmanager/related_images_test.go index 3727dbfc3..afbac59ff 100644 --- a/pkg/controller/certmanager/related_images_test.go +++ b/pkg/controller/certmanager/related_images_test.go @@ -42,3 +42,108 @@ func Test_certManagerImage(t *testing.T) { }) } } + +func Test_certManagerWebhookImage(t *testing.T) { + tests := []struct { + name string + defaultImage string + envVarValue string + want string + }{ + { + name: "Use default webhook image when env var is empty", + defaultImage: "quay.io/jetstack/cert-manager-webhook:latest", + envVarValue: "", + want: "quay.io/jetstack/cert-manager-webhook:latest", + }, + { + name: "Use override webhook image when env var is set", + defaultImage: "quay.io/jetstack/cert-manager-webhook:latest", + envVarValue: "registry.redhat.io/cert-manager/cert-manager-webhook-rhel-8:latest", + want: "registry.redhat.io/cert-manager/cert-manager-webhook-rhel-8:latest", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + os.Setenv("RELATED_IMAGE_CERT_MANAGER_WEBHOOK", tt.envVarValue) + defer os.Unsetenv("RELATED_IMAGE_CERT_MANAGER_WEBHOOK") + if got := certManagerImage(tt.defaultImage); got != tt.want { + t.Errorf("certManagerImage() = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_certManagerCAInjectorImage(t *testing.T) { + tests := []struct { + name string + defaultImage string + envVarValue string + want string + }{ + { + name: "Use default cainjector image when env var is empty", + defaultImage: "quay.io/jetstack/cert-manager-cainjector:latest", + envVarValue: "", + want: "quay.io/jetstack/cert-manager-cainjector:latest", + }, + { + name: "Use override cainjector image when env var is set", + defaultImage: "quay.io/jetstack/cert-manager-cainjector:latest", + envVarValue: "registry.redhat.io/cert-manager/cert-manager-cainjector-rhel-8:latest", + want: "registry.redhat.io/cert-manager/cert-manager-cainjector-rhel-8:latest", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + os.Setenv("RELATED_IMAGE_CERT_MANAGER_CA_INJECTOR", tt.envVarValue) + defer os.Unsetenv("RELATED_IMAGE_CERT_MANAGER_CA_INJECTOR") + if got := certManagerImage(tt.defaultImage); got != tt.want { + t.Errorf("certManagerImage() = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_certManagerACMESolverImage(t *testing.T) { + tests := []struct { + name string + defaultImage string + envVarValue string + want string + }{ + { + name: "Use default acmesolver image when env var is empty", + defaultImage: "quay.io/jetstack/cert-manager-acmesolver:latest", + envVarValue: "", + want: "quay.io/jetstack/cert-manager-acmesolver:latest", + }, + { + name: "Use override acmesolver image when env var is set", + defaultImage: "quay.io/jetstack/cert-manager-acmesolver:latest", + envVarValue: "registry.redhat.io/cert-manager/cert-manager-acmesolver-rhel-8:latest", + want: "registry.redhat.io/cert-manager/cert-manager-acmesolver-rhel-8:latest", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + os.Setenv("RELATED_IMAGE_CERT_MANAGER_ACMESOLVER", tt.envVarValue) + defer os.Unsetenv("RELATED_IMAGE_CERT_MANAGER_ACMESOLVER") + if got := certManagerImage(tt.defaultImage); got != tt.want { + t.Errorf("certManagerImage() = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_certManagerImageUnknownImage(t *testing.T) { + // An unknown image should be returned as-is regardless of env vars + os.Setenv("RELATED_IMAGE_CERT_MANAGER_CONTROLLER", "override:latest") + defer os.Unsetenv("RELATED_IMAGE_CERT_MANAGER_CONTROLLER") + + defaultImage := "quay.io/some-other/image:v1.0" + got := certManagerImage(defaultImage) + if got != defaultImage { + t.Errorf("certManagerImage() = %v, want %v", got, defaultImage) + } +} diff --git a/pkg/controller/common/client_test.go b/pkg/controller/common/client_test.go index d0ea892ba..092660d94 100644 --- a/pkg/controller/common/client_test.go +++ b/pkg/controller/common/client_test.go @@ -2,15 +2,20 @@ package common import ( "context" + "fmt" "net/http" "testing" "github.com/go-logr/logr" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + 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/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/rest" "k8s.io/client-go/tools/events" "k8s.io/client-go/tools/record" @@ -134,3 +139,353 @@ func TestNewClient(t *testing.T) { require.True(t, ok, "NewClient must return *ctrlClientImpl") assert.True(t, impl.Client == cl, "wrapped client must be the exact manager client instance") } + +// mockClient implements client.Client with configurable behavior for testing. +type mockClient struct { + client.Client // embed to satisfy interface; only tested methods are overridden + + getFunc func(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error + createFunc func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error + updateFunc func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error + deleteFunc func(ctx context.Context, obj client.Object, opts ...client.DeleteOption) error + listFunc func(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error + patchFunc func(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error + + statusClient *mockStatusClient +} + +type mockStatusClient struct { + updateFunc func(ctx context.Context, obj client.Object, opts ...client.SubResourceUpdateOption) error +} + +func (m *mockStatusClient) Update(ctx context.Context, obj client.Object, opts ...client.SubResourceUpdateOption) error { + if m.updateFunc != nil { + return m.updateFunc(ctx, obj, opts...) + } + return nil +} + +func (m *mockStatusClient) Create(ctx context.Context, obj client.Object, subResource client.Object, opts ...client.SubResourceCreateOption) error { + return nil +} + +func (m *mockStatusClient) Patch(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.SubResourcePatchOption) error { + return nil +} + +func (m *mockStatusClient) Apply(ctx context.Context, cfg runtime.ApplyConfiguration, opts ...client.SubResourceApplyOption) error { + return nil +} + +func (m *mockClient) Get(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { + if m.getFunc != nil { + return m.getFunc(ctx, key, obj, opts...) + } + return nil +} + +func (m *mockClient) Create(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { + if m.createFunc != nil { + return m.createFunc(ctx, obj, opts...) + } + return nil +} + +func (m *mockClient) Update(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { + if m.updateFunc != nil { + return m.updateFunc(ctx, obj, opts...) + } + return nil +} + +func (m *mockClient) Delete(ctx context.Context, obj client.Object, opts ...client.DeleteOption) error { + if m.deleteFunc != nil { + return m.deleteFunc(ctx, obj, opts...) + } + return nil +} + +func (m *mockClient) List(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error { + if m.listFunc != nil { + return m.listFunc(ctx, list, opts...) + } + return nil +} + +func (m *mockClient) Patch(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error { + if m.patchFunc != nil { + return m.patchFunc(ctx, obj, patch, opts...) + } + return nil +} + +func (m *mockClient) Status() client.SubResourceWriter { + if m.statusClient != nil { + return m.statusClient + } + return &mockStatusClient{} +} + +func newCtrlClient(mc *mockClient) *ctrlClientImpl { + return &ctrlClientImpl{Client: mc} +} + +func TestClientExists_Found(t *testing.T) { + mc := &mockClient{ + getFunc: func(_ context.Context, _ client.ObjectKey, _ client.Object, _ ...client.GetOption) error { + return nil + }, + } + c := newCtrlClient(mc) + + found, err := c.Exists(context.Background(), types.NamespacedName{Name: "test", Namespace: "default"}, &corev1.ConfigMap{}) + require.NoError(t, err) + assert.True(t, found) +} + +func TestClientExists_NotFound(t *testing.T) { + mc := &mockClient{ + getFunc: func(_ context.Context, _ client.ObjectKey, _ client.Object, _ ...client.GetOption) error { + return apierrors.NewNotFound(schema.GroupResource{Resource: "configmaps"}, "missing") + }, + } + c := newCtrlClient(mc) + + found, err := c.Exists(context.Background(), types.NamespacedName{Name: "missing", Namespace: "default"}, &corev1.ConfigMap{}) + require.NoError(t, err) + assert.False(t, found) +} + +func TestClientExists_Error(t *testing.T) { + mc := &mockClient{ + getFunc: func(_ context.Context, _ client.ObjectKey, _ client.Object, _ ...client.GetOption) error { + return fmt.Errorf("connection refused") + }, + } + c := newCtrlClient(mc) + + found, err := c.Exists(context.Background(), types.NamespacedName{Name: "test", Namespace: "default"}, &corev1.ConfigMap{}) + require.Error(t, err) + assert.False(t, found) +} + +func TestClientGet_Success(t *testing.T) { + mc := &mockClient{ + getFunc: func(_ context.Context, key client.ObjectKey, obj client.Object, _ ...client.GetOption) error { + cm := obj.(*corev1.ConfigMap) + cm.Name = key.Name + cm.Namespace = key.Namespace + cm.Data = map[string]string{"k": "v"} + return nil + }, + } + c := newCtrlClient(mc) + + cm := &corev1.ConfigMap{} + err := c.Get(context.Background(), types.NamespacedName{Name: "myconfig", Namespace: "default"}, cm) + require.NoError(t, err) + assert.Equal(t, "myconfig", cm.Name) + assert.Equal(t, "v", cm.Data["k"]) +} + +func TestClientGet_NotFound(t *testing.T) { + mc := &mockClient{ + getFunc: func(_ context.Context, _ client.ObjectKey, _ client.Object, _ ...client.GetOption) error { + return apierrors.NewNotFound(schema.GroupResource{Resource: "configmaps"}, "missing") + }, + } + c := newCtrlClient(mc) + + err := c.Get(context.Background(), types.NamespacedName{Name: "missing", Namespace: "default"}, &corev1.ConfigMap{}) + require.Error(t, err) + assert.Contains(t, err.Error(), "failed to get object") +} + +func TestClientCreate_Success(t *testing.T) { + var created bool + mc := &mockClient{ + createFunc: func(_ context.Context, _ client.Object, _ ...client.CreateOption) error { + created = true + return nil + }, + } + c := newCtrlClient(mc) + + cm := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "new", Namespace: "default"}} + err := c.Create(context.Background(), cm) + require.NoError(t, err) + assert.True(t, created) +} + +func TestClientCreate_AlreadyExists(t *testing.T) { + mc := &mockClient{ + createFunc: func(_ context.Context, _ client.Object, _ ...client.CreateOption) error { + return apierrors.NewAlreadyExists(schema.GroupResource{Resource: "configmaps"}, "dup") + }, + } + c := newCtrlClient(mc) + + err := c.Create(context.Background(), &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "dup", Namespace: "default"}}) + require.Error(t, err) + assert.Contains(t, err.Error(), "failed to create object") +} + +func TestClientUpdate_Success(t *testing.T) { + var updated bool + mc := &mockClient{ + updateFunc: func(_ context.Context, _ client.Object, _ ...client.UpdateOption) error { + updated = true + return nil + }, + } + c := newCtrlClient(mc) + + cm := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "upd", Namespace: "default"}} + err := c.Update(context.Background(), cm) + require.NoError(t, err) + assert.True(t, updated) +} + +func TestClientDelete_Success(t *testing.T) { + var deleted bool + mc := &mockClient{ + deleteFunc: func(_ context.Context, _ client.Object, _ ...client.DeleteOption) error { + deleted = true + return nil + }, + } + c := newCtrlClient(mc) + + cm := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "del", Namespace: "default"}} + err := c.Delete(context.Background(), cm) + require.NoError(t, err) + assert.True(t, deleted) +} + +func TestClientList_Success(t *testing.T) { + mc := &mockClient{ + listFunc: func(_ context.Context, list client.ObjectList, _ ...client.ListOption) error { + cmList := list.(*corev1.ConfigMapList) + cmList.Items = []corev1.ConfigMap{ + {ObjectMeta: metav1.ObjectMeta{Name: "cm1"}}, + {ObjectMeta: metav1.ObjectMeta{Name: "cm2"}}, + } + return nil + }, + } + c := newCtrlClient(mc) + + list := &corev1.ConfigMapList{} + err := c.List(context.Background(), list) + require.NoError(t, err) + assert.Len(t, list.Items, 2) +} + +func TestClientUpdateWithRetry_Success(t *testing.T) { + updateCallCount := 0 + mc := &mockClient{ + getFunc: func(_ context.Context, _ client.ObjectKey, obj client.Object, _ ...client.GetOption) error { + obj.SetResourceVersion("123") + return nil + }, + updateFunc: func(_ context.Context, obj client.Object, _ ...client.UpdateOption) error { + updateCallCount++ + // Succeed on the first try. + return nil + }, + } + c := newCtrlClient(mc) + + cm := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "retry-cm", Namespace: "default"}} + err := c.UpdateWithRetry(context.Background(), cm) + require.NoError(t, err) + assert.Equal(t, 1, updateCallCount) + assert.Equal(t, "123", cm.GetResourceVersion()) +} + +func TestClientUpdateWithRetry_ConflictThenSuccess(t *testing.T) { + callCount := 0 + mc := &mockClient{ + getFunc: func(_ context.Context, _ client.ObjectKey, obj client.Object, _ ...client.GetOption) error { + obj.SetResourceVersion(fmt.Sprintf("rv-%d", callCount)) + return nil + }, + updateFunc: func(_ context.Context, _ client.Object, _ ...client.UpdateOption) error { + callCount++ + if callCount == 1 { + return apierrors.NewConflict(schema.GroupResource{Resource: "configmaps"}, "retry-cm", fmt.Errorf("conflict")) + } + return nil + }, + } + c := newCtrlClient(mc) + + cm := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "retry-cm", Namespace: "default"}} + err := c.UpdateWithRetry(context.Background(), cm) + require.NoError(t, err) + assert.Equal(t, 2, callCount, "update should be called twice: conflict then success") +} + +func TestClientPatch_Success(t *testing.T) { + var patched bool + mc := &mockClient{ + patchFunc: func(_ context.Context, _ client.Object, _ client.Patch, _ ...client.PatchOption) error { + patched = true + return nil + }, + } + c := newCtrlClient(mc) + + cm := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "patch-cm", Namespace: "default"}} + err := c.Patch(context.Background(), cm, client.MergeFrom(cm)) + require.NoError(t, err) + assert.True(t, patched) +} + +func TestClientPatch_Error(t *testing.T) { + mc := &mockClient{ + patchFunc: func(_ context.Context, _ client.Object, _ client.Patch, _ ...client.PatchOption) error { + return fmt.Errorf("patch failed") + }, + } + c := newCtrlClient(mc) + + cm := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "patch-cm", Namespace: "default"}} + err := c.Patch(context.Background(), cm, client.MergeFrom(cm)) + require.Error(t, err) + assert.Contains(t, err.Error(), "failed to patch object") +} + +func TestClientStatusUpdate_Success(t *testing.T) { + var updated bool + mc := &mockClient{ + statusClient: &mockStatusClient{ + updateFunc: func(_ context.Context, _ client.Object, _ ...client.SubResourceUpdateOption) error { + updated = true + return nil + }, + }, + } + c := newCtrlClient(mc) + + cm := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "status-cm", Namespace: "default"}} + err := c.StatusUpdate(context.Background(), cm) + require.NoError(t, err) + assert.True(t, updated) +} + +func TestClientStatusUpdate_Error(t *testing.T) { + mc := &mockClient{ + statusClient: &mockStatusClient{ + updateFunc: func(_ context.Context, _ client.Object, _ ...client.SubResourceUpdateOption) error { + return fmt.Errorf("status update failed") + }, + }, + } + c := newCtrlClient(mc) + + cm := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "status-cm", Namespace: "default"}} + err := c.StatusUpdate(context.Background(), cm) + require.Error(t, err) + assert.Contains(t, err.Error(), "failed to update status") +} diff --git a/pkg/controller/common/reconcile_result_test.go b/pkg/controller/common/reconcile_result_test.go new file mode 100644 index 000000000..856d603aa --- /dev/null +++ b/pkg/controller/common/reconcile_result_test.go @@ -0,0 +1,162 @@ +package common + +import ( + "fmt" + "testing" + "time" + + "github.com/go-logr/logr" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + ctrl "sigs.k8s.io/controller-runtime" + + v1alpha1 "github.com/openshift/cert-manager-operator/api/operator/v1alpha1" +) + +func TestHandleReconcileResult(t *testing.T) { + const requeueDuration = 5 * time.Second + log := logr.Discard() + + tests := map[string]struct { + reconcileErr error + updateFnErr error + expectResult ctrl.Result + expectErr bool + expectCallCount int + }{ + "irrecoverable error sets degraded and does not requeue": { + reconcileErr: NewIrrecoverableError(fmt.Errorf("perm fail"), "something broke"), + expectResult: ctrl.Result{}, + expectErr: false, + expectCallCount: 1, + }, + "recoverable error sets progressing and requeues after duration": { + reconcileErr: NewRetryRequiredError(fmt.Errorf("temp fail"), "retry later"), + expectResult: ctrl.Result{RequeueAfter: requeueDuration}, + expectErr: false, + expectCallCount: 1, + }, + "nil error sets ready and does not requeue": { + reconcileErr: nil, + expectResult: ctrl.Result{}, + expectErr: false, + expectCallCount: 1, + }, + "updateConditionFn error on irrecoverable is propagated": { + reconcileErr: NewIrrecoverableError(fmt.Errorf("perm fail"), "broke"), + updateFnErr: fmt.Errorf("status update failed"), + expectResult: ctrl.Result{}, + expectErr: true, + expectCallCount: 1, + }, + "updateConditionFn error on recoverable is propagated instead of requeue": { + reconcileErr: NewRetryRequiredError(fmt.Errorf("temp fail"), "retry"), + updateFnErr: fmt.Errorf("status update failed"), + expectResult: ctrl.Result{}, + expectErr: true, + expectCallCount: 1, + }, + "updateConditionFn error on success is propagated": { + reconcileErr: nil, + updateFnErr: fmt.Errorf("status update failed"), + expectResult: ctrl.Result{}, + expectErr: true, + expectCallCount: 1, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + + // Fresh status so conditions are always unset and SetCondition returns true. + status := &v1alpha1.ConditionalStatus{} + + callCount := 0 + updateFn := func(prependErr error) error { + callCount++ + return tc.updateFnErr + } + + result, err := HandleReconcileResult(status, tc.reconcileErr, log, updateFn, requeueDuration) + + assert.Equal(t, tc.expectResult, result) + if tc.expectErr { + require.Error(t, err) + } else { + require.NoError(t, err) + } + assert.Equal(t, tc.expectCallCount, callCount, "updateConditionFn call count mismatch") + }) + } +} + +func TestHandleReconcileResult_ConditionsSet(t *testing.T) { + const requeueDuration = 5 * time.Second + log := logr.Discard() + + noopUpdate := func(_ error) error { return nil } + + t.Run("irrecoverable error sets Degraded=True and Ready=False", func(t *testing.T) { + status := &v1alpha1.ConditionalStatus{} + _, _ = HandleReconcileResult(status, NewIrrecoverableError(fmt.Errorf("fail"), "broke"), log, noopUpdate, requeueDuration) + + degraded := status.GetCondition(v1alpha1.Degraded) + require.NotNil(t, degraded) + assert.Equal(t, "True", string(degraded.Status)) + assert.Equal(t, v1alpha1.ReasonFailed, degraded.Reason) + + ready := status.GetCondition(v1alpha1.Ready) + require.NotNil(t, ready) + assert.Equal(t, "False", string(ready.Status)) + }) + + t.Run("recoverable error sets Degraded=False and Ready=False", func(t *testing.T) { + status := &v1alpha1.ConditionalStatus{} + _, _ = HandleReconcileResult(status, NewRetryRequiredError(fmt.Errorf("temp"), "retry"), log, noopUpdate, requeueDuration) + + degraded := status.GetCondition(v1alpha1.Degraded) + require.NotNil(t, degraded) + assert.Equal(t, "False", string(degraded.Status)) + + ready := status.GetCondition(v1alpha1.Ready) + require.NotNil(t, ready) + assert.Equal(t, "False", string(ready.Status)) + assert.Equal(t, v1alpha1.ReasonInProgress, ready.Reason) + }) + + t.Run("success sets Degraded=False and Ready=True", func(t *testing.T) { + status := &v1alpha1.ConditionalStatus{} + _, _ = HandleReconcileResult(status, nil, log, noopUpdate, requeueDuration) + + degraded := status.GetCondition(v1alpha1.Degraded) + require.NotNil(t, degraded) + assert.Equal(t, "False", string(degraded.Status)) + + ready := status.GetCondition(v1alpha1.Ready) + require.NotNil(t, ready) + assert.Equal(t, "True", string(ready.Status)) + assert.Equal(t, v1alpha1.ReasonReady, ready.Reason) + }) +} + +func TestHandleReconcileResult_NoUpdateWhenConditionsUnchanged(t *testing.T) { + const requeueDuration = 5 * time.Second + log := logr.Discard() + + // Pre-set conditions to match what success would set. + status := &v1alpha1.ConditionalStatus{} + status.SetCondition(v1alpha1.Degraded, "False", v1alpha1.ReasonReady, "") + status.SetCondition(v1alpha1.Ready, "True", v1alpha1.ReasonReady, "reconciliation successful") + + callCount := 0 + updateFn := func(_ error) error { + callCount++ + return nil + } + + result, err := HandleReconcileResult(status, nil, log, updateFn, requeueDuration) + require.NoError(t, err) + assert.Equal(t, ctrl.Result{}, result) + assert.Equal(t, 0, callCount, "updateConditionFn should not be called when conditions are unchanged") +} diff --git a/pkg/controller/common/utils_test.go b/pkg/controller/common/utils_test.go index d4117d34d..0368921ab 100644 --- a/pkg/controller/common/utils_test.go +++ b/pkg/controller/common/utils_test.go @@ -8,9 +8,18 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/runtime/serializer" "sigs.k8s.io/controller-runtime/pkg/client" ) +func TestUpdateName(t *testing.T) { + cm := &corev1.ConfigMap{} + UpdateName(cm, "test-name") + assert.Equal(t, "test-name", cm.Name) +} + // TestUpdateNamespace provides table-driven tests for UpdateNamespace(obj, newNamespace). func TestUpdateNamespace(t *testing.T) { tests := []struct { @@ -292,3 +301,36 @@ func TestAddAnnotation(t *testing.T) { }) } } + +func TestDecodeObjBytes_ValidConfigMap(t *testing.T) { + scheme := runtime.NewScheme() + _ = corev1.AddToScheme(scheme) + codecs := serializer.NewCodecFactory(scheme) + gv := schema.GroupVersion{Group: "", Version: "v1"} + + yamlBytes := []byte(`apiVersion: v1 +kind: ConfigMap +metadata: + name: decoded-cm + namespace: test-ns +data: + key: value +`) + + cm := DecodeObjBytes[*corev1.ConfigMap](codecs, gv, yamlBytes) + require.NotNil(t, cm) + assert.Equal(t, "decoded-cm", cm.Name) + assert.Equal(t, "test-ns", cm.Namespace) + assert.Equal(t, "value", cm.Data["key"]) +} + +func TestDecodeObjBytes_InvalidBytes(t *testing.T) { + scheme := runtime.NewScheme() + _ = corev1.AddToScheme(scheme) + codecs := serializer.NewCodecFactory(scheme) + gv := schema.GroupVersion{Group: "", Version: "v1"} + + require.Panics(t, func() { + DecodeObjBytes[*corev1.ConfigMap](codecs, gv, []byte("not valid yaml or json {{{")) + }) +} diff --git a/pkg/controller/common/validation_test.go b/pkg/controller/common/validation_test.go new file mode 100644 index 000000000..949e23af5 --- /dev/null +++ b/pkg/controller/common/validation_test.go @@ -0,0 +1,214 @@ +package common + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + "k8s.io/apimachinery/pkg/util/validation/field" +) + +func TestValidateLabelsConfig(t *testing.T) { + fldPath := field.NewPath("spec") + + t.Run("valid labels pass", func(t *testing.T) { + labels := map[string]string{ + "app": "test", + "example.com/my-component": "frontend", + } + err := ValidateLabelsConfig(labels, fldPath) + require.NoError(t, err) + }) + + t.Run("invalid label key returns error", func(t *testing.T) { + labels := map[string]string{ + "INVALID KEY WITH SPACES": "value", + } + err := ValidateLabelsConfig(labels, fldPath) + require.Error(t, err) + }) + + t.Run("empty labels pass", func(t *testing.T) { + err := ValidateLabelsConfig(map[string]string{}, fldPath) + require.NoError(t, err) + }) +} + +func TestValidateAnnotationsConfig(t *testing.T) { + fldPath := field.NewPath("spec") + + t.Run("valid annotations pass", func(t *testing.T) { + annotations := map[string]string{ + "kubectl.kubernetes.io/last-applied-configuration": "{}", + "my-annotation": "some-value", + } + err := ValidateAnnotationsConfig(annotations, fldPath) + require.NoError(t, err) + }) + + t.Run("invalid annotation key returns error", func(t *testing.T) { + annotations := map[string]string{ + "invalid key!@#$": "value", + } + err := ValidateAnnotationsConfig(annotations, fldPath) + require.Error(t, err) + }) +} + +func TestValidateNodeSelectorConfig(t *testing.T) { + fldPath := field.NewPath("spec") + + t.Run("valid nodeSelector passes", func(t *testing.T) { + nodeSelector := map[string]string{ + "kubernetes.io/os": "linux", + "node-role": "worker", + } + err := ValidateNodeSelectorConfig(nodeSelector, fldPath) + require.NoError(t, err) + }) + + t.Run("empty value key passes", func(t *testing.T) { + nodeSelector := map[string]string{ + "node.kubernetes.io/instance-type": "", + } + err := ValidateNodeSelectorConfig(nodeSelector, fldPath) + require.NoError(t, err) + }) + + t.Run("invalid key returns error", func(t *testing.T) { + nodeSelector := map[string]string{ + "BAD KEY!!!": "value", + } + err := ValidateNodeSelectorConfig(nodeSelector, fldPath) + require.Error(t, err) + }) +} + +func TestValidateTolerationsConfig(t *testing.T) { + fldPath := field.NewPath("spec") + + t.Run("valid tolerations pass", func(t *testing.T) { + tolerations := []corev1.Toleration{ + { + Key: "node.kubernetes.io/not-ready", + Operator: corev1.TolerationOpExists, + Effect: corev1.TaintEffectNoSchedule, + }, + { + Key: "dedicated", + Operator: corev1.TolerationOpEqual, + Value: "cert-manager", + Effect: corev1.TaintEffectNoSchedule, + }, + } + err := ValidateTolerationsConfig(tolerations, fldPath) + require.NoError(t, err) + }) + + t.Run("invalid operator returns error", func(t *testing.T) { + tolerations := []corev1.Toleration{ + { + Key: "key", + Operator: corev1.TolerationOperator("InvalidOp"), + Effect: corev1.TaintEffectNoSchedule, + }, + } + err := ValidateTolerationsConfig(tolerations, fldPath) + require.Error(t, err) + }) + + t.Run("empty tolerations pass", func(t *testing.T) { + err := ValidateTolerationsConfig([]corev1.Toleration{}, fldPath) + require.NoError(t, err) + }) +} + +func TestValidateResourceRequirements(t *testing.T) { + fldPath := field.NewPath("spec") + + t.Run("valid cpu and memory pass", func(t *testing.T) { + reqs := corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("100m"), + corev1.ResourceMemory: resource.MustParse("128Mi"), + }, + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("500m"), + corev1.ResourceMemory: resource.MustParse("512Mi"), + }, + } + err := ValidateResourceRequirements(reqs, fldPath) + require.NoError(t, err) + }) + + t.Run("negative cpu returns error", func(t *testing.T) { + reqs := corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("-100m"), + }, + } + err := ValidateResourceRequirements(reqs, fldPath) + require.Error(t, err) + }) + + t.Run("empty requirements pass", func(t *testing.T) { + err := ValidateResourceRequirements(corev1.ResourceRequirements{}, fldPath) + require.NoError(t, err) + }) +} + +func TestValidateAffinityRules(t *testing.T) { + fldPath := field.NewPath("spec") + + t.Run("nil affinity passes", func(t *testing.T) { + err := ValidateAffinityRules(nil, fldPath) + require.NoError(t, err) + }) + + t.Run("valid node affinity passes", func(t *testing.T) { + affinity := &corev1.Affinity{ + NodeAffinity: &corev1.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &corev1.NodeSelector{ + NodeSelectorTerms: []corev1.NodeSelectorTerm{ + { + MatchExpressions: []corev1.NodeSelectorRequirement{ + { + Key: "kubernetes.io/os", + Operator: corev1.NodeSelectorOpIn, + Values: []string{"linux"}, + }, + }, + }, + }, + }, + }, + } + err := ValidateAffinityRules(affinity, fldPath) + require.NoError(t, err) + }) + + t.Run("invalid label selector in node affinity returns error", func(t *testing.T) { + affinity := &corev1.Affinity{ + NodeAffinity: &corev1.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &corev1.NodeSelector{ + NodeSelectorTerms: []corev1.NodeSelectorTerm{ + { + MatchExpressions: []corev1.NodeSelectorRequirement{ + { + Key: "kubernetes.io/os", + Operator: corev1.NodeSelectorOperator("BadOp"), + Values: []string{"linux"}, + }, + }, + }, + }, + }, + }, + } + err := ValidateAffinityRules(affinity, fldPath) + require.Error(t, err) + assert.Contains(t, err.Error(), "not a valid selector operator") + }) +} diff --git a/pkg/controller/istiocsr/networkpolicies_test.go b/pkg/controller/istiocsr/networkpolicies_test.go new file mode 100644 index 000000000..7a803ce48 --- /dev/null +++ b/pkg/controller/istiocsr/networkpolicies_test.go @@ -0,0 +1,225 @@ +package istiocsr + +import ( + "context" + "strings" + "testing" + + networkingv1 "k8s.io/api/networking/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/openshift/cert-manager-operator/pkg/controller/common/fakes" +) + +func TestCreateOrApplyNetworkPolicies(t *testing.T) { + tests := []struct { + name string + preReq func(*Reconciler, *fakes.FakeCtrlClient) + wantErr string + }{ + { + name: "happy path all network policies applied", + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + // Exists returns false so Create is called, Create succeeds by default + }, + }, + { + name: "create error propagates", + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + m.CreateCalls(func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { + switch obj.(type) { + case *networkingv1.NetworkPolicy: + return errTestClient + } + return nil + }) + }, + wantErr: "failed to create/update network policy from", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r := testReconciler(t) + mock := &fakes.FakeCtrlClient{} + if tt.preReq != nil { + tt.preReq(r, mock) + } + r.CtrlClient = mock + istiocsr := testIstioCSR() + + err := r.createOrApplyNetworkPolicies(istiocsr, controllerDefaultResourceLabels, false) + if tt.wantErr != "" { + if err == nil { + t.Fatalf("expected error containing %q, got nil", tt.wantErr) + } + if got := err.Error(); !strings.Contains(got, tt.wantErr) { + t.Errorf("createOrApplyNetworkPolicies() err = %q, want substring %q", got, tt.wantErr) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + // Verify Create was called for each network policy asset + if mock.CreateCallCount() != len(istioCSRNetworkPolicyAssets) { + t.Errorf("expected %d Create calls, got %d", len(istioCSRNetworkPolicyAssets), mock.CreateCallCount()) + } + }) + } +} + +func TestCreateOrUpdateNetworkPolicy(t *testing.T) { + tests := []struct { + name string + istioCSRCreateRecon bool + preReq func(*Reconciler, *fakes.FakeCtrlClient) + wantErr string + assertCalls func(t *testing.T, mock *fakes.FakeCtrlClient) + }{ + { + name: "policy does not exist creates it", + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + return false, nil + }) + }, + assertCalls: func(t *testing.T, mock *fakes.FakeCtrlClient) { + if mock.CreateCallCount() != 1 { + t.Errorf("expected 1 Create call, got %d", mock.CreateCallCount()) + } + }, + }, + { + name: "policy exists and matches is a no-op", + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + switch o := obj.(type) { + case *networkingv1.NetworkPolicy: + np := testNetworkPolicy() + np.DeepCopyInto(o) + } + return true, nil + }) + }, + assertCalls: func(t *testing.T, mock *fakes.FakeCtrlClient) { + if mock.CreateCallCount() != 0 { + t.Errorf("expected 0 Create calls for matching policy, got %d", mock.CreateCallCount()) + } + if mock.UpdateWithRetryCallCount() != 0 { + t.Errorf("expected 0 UpdateWithRetry calls for matching policy, got %d", mock.UpdateWithRetryCallCount()) + } + }, + }, + { + name: "policy exists but differs triggers update", + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + switch o := obj.(type) { + case *networkingv1.NetworkPolicy: + np := testNetworkPolicy() + // Modify labels so hasObjectChanged returns true + np.SetLabels(map[string]string{"modified": "true"}) + np.DeepCopyInto(o) + } + return true, nil + }) + m.UpdateWithRetryReturns(nil) + }, + assertCalls: func(t *testing.T, mock *fakes.FakeCtrlClient) { + if mock.UpdateWithRetryCallCount() != 1 { + t.Errorf("expected 1 UpdateWithRetry call for modified policy, got %d", mock.UpdateWithRetryCallCount()) + } + }, + }, + { + name: "Exists error propagates", + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + return false, errTestClient + }) + }, + wantErr: "failed to check", + }, + { + name: "Create error propagates", + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + return false, nil + }) + m.CreateCalls(func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { + return errTestClient + }) + }, + wantErr: "failed to create", + }, + { + name: "UpdateWithRetry error propagates", + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + switch o := obj.(type) { + case *networkingv1.NetworkPolicy: + np := testNetworkPolicy() + np.SetLabels(map[string]string{"modified": "true"}) + np.DeepCopyInto(o) + } + return true, nil + }) + m.UpdateWithRetryCalls(func(ctx context.Context, obj client.Object, option ...client.UpdateOption) error { + return errTestClient + }) + }, + wantErr: "failed to update", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r := testReconciler(t) + mock := &fakes.FakeCtrlClient{} + if tt.preReq != nil { + tt.preReq(r, mock) + } + r.CtrlClient = mock + + policy := testNetworkPolicy() + err := r.createOrUpdateNetworkPolicy(policy, tt.istioCSRCreateRecon) + + if tt.wantErr != "" { + if err == nil { + t.Fatalf("expected error containing %q, got nil", tt.wantErr) + } + if got := err.Error(); !strings.Contains(got, tt.wantErr) { + t.Errorf("createOrUpdateNetworkPolicy() err = %q, want substring %q", got, tt.wantErr) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if tt.assertCalls != nil { + tt.assertCalls(t, mock) + } + }) + } +} + +// testNetworkPolicy returns a NetworkPolicy with expected labels and spec for tests. +func testNetworkPolicy() *networkingv1.NetworkPolicy { + return &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-network-policy", + Namespace: testIstioCSRNamespace, + Labels: controllerDefaultResourceLabels, + }, + Spec: networkingv1.NetworkPolicySpec{ + PolicyTypes: []networkingv1.PolicyType{networkingv1.PolicyTypeIngress}, + PodSelector: metav1.LabelSelector{}, + }, + } +} + + diff --git a/pkg/controller/istiocsr/utils_test.go b/pkg/controller/istiocsr/utils_test.go index d141e20bc..9409a2682 100644 --- a/pkg/controller/istiocsr/utils_test.go +++ b/pkg/controller/istiocsr/utils_test.go @@ -13,6 +13,10 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" certmanagerv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" + certmanagermetav1 "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" + + "github.com/openshift/cert-manager-operator/api/operator/v1alpha1" + "github.com/openshift/cert-manager-operator/pkg/controller/common/fakes" ) // baseDeployment returns a minimal deployment for spec comparison tests. @@ -529,3 +533,208 @@ func TestNetworkPolicySpecModified(t *testing.T) { }) } } + +func TestValidateIstioCSRConfig(t *testing.T) { + tests := []struct { + name string + istiocsr *v1alpha1.IstioCSR + wantErr bool + wantErrMsg string + }{ + { + name: "empty IstioCSRConfig", + istiocsr: &v1alpha1.IstioCSR{ + Spec: v1alpha1.IstioCSRSpec{}, + }, + wantErr: true, + wantErrMsg: "istioCSRConfig config cannot be empty", + }, + { + name: "empty IstiodTLSConfig", + istiocsr: &v1alpha1.IstioCSR{ + Spec: v1alpha1.IstioCSRSpec{ + IstioCSRConfig: v1alpha1.IstioCSRConfig{ + CertManager: v1alpha1.CertManagerConfig{ + IssuerRef: certmanagermetav1.ObjectReference{ + Name: "test", + Kind: "issuer", + }, + }, + Istio: v1alpha1.IstioConfig{ + Namespace: "istio-system", + Revisions: []string{"default"}, + }, + }, + }, + }, + wantErr: true, + wantErrMsg: "istiodTLSConfig config cannot be empty", + }, + { + name: "empty Istio config", + istiocsr: &v1alpha1.IstioCSR{ + Spec: v1alpha1.IstioCSRSpec{ + IstioCSRConfig: v1alpha1.IstioCSRConfig{ + IstiodTLSConfig: v1alpha1.IstiodTLSConfig{ + TrustDomain: "cluster.local", + PrivateKeySize: 2048, + CertificateDuration: &metav1.Duration{Duration: DefaultCertificateDuration}, + }, + CertManager: v1alpha1.CertManagerConfig{ + IssuerRef: certmanagermetav1.ObjectReference{ + Name: "test", + Kind: "issuer", + }, + }, + }, + }, + }, + wantErr: true, + wantErrMsg: "istio config cannot be empty", + }, + { + name: "empty CertManager config", + istiocsr: &v1alpha1.IstioCSR{ + Spec: v1alpha1.IstioCSRSpec{ + IstioCSRConfig: v1alpha1.IstioCSRConfig{ + IstiodTLSConfig: v1alpha1.IstiodTLSConfig{ + TrustDomain: "cluster.local", + PrivateKeySize: 2048, + CertificateDuration: &metav1.Duration{Duration: DefaultCertificateDuration}, + }, + Istio: v1alpha1.IstioConfig{ + Namespace: "istio-system", + Revisions: []string{"default"}, + }, + }, + }, + }, + wantErr: true, + wantErrMsg: "certManager config cannot be empty", + }, + { + name: "valid config passes", + istiocsr: testIstioCSR(), + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateIstioCSRConfig(tt.istiocsr) + if tt.wantErr { + if err == nil { + t.Fatal("expected error, got nil") + } + if !strings.Contains(err.Error(), tt.wantErrMsg) { + t.Errorf("validateIstioCSRConfig() err = %q, want substring %q", err.Error(), tt.wantErrMsg) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + } +} + +func TestUpdateCondition(t *testing.T) { + tests := []struct { + name string + prependErr error + setupFake func(*fakes.FakeCtrlClient) + wantErr bool + wantErrMsg string + // checkBug verifies that the aggregate error contains prependErr when both + // prependErr is non-nil and status update fails. The current implementation + // has a bug: it uses `err` (the raw status update error) instead of + // `prependErr` in utilerrors.NewAggregate, so the original reconcile error + // is lost from the aggregate. Compare with pkg/controller/http01proxy/utils.go + // which correctly uses `prependErr`. + checkBuggyAggregate bool + }{ + { + name: "prependErr nil and status update succeeds returns nil", + prependErr: nil, + wantErr: false, + }, + { + name: "prependErr non-nil and status update succeeds returns prependErr", + prependErr: fmt.Errorf("original reconcile error"), + wantErr: true, + wantErrMsg: "original reconcile error", + }, + { + name: "prependErr nil and status update fails returns update error", + prependErr: nil, + setupFake: func(fc *fakes.FakeCtrlClient) { + fc.StatusUpdateReturns(fmt.Errorf("status update conflict")) + }, + wantErr: true, + wantErrMsg: "failed to update", + }, + { + name: "prependErr non-nil and status update fails returns aggregate error", + prependErr: fmt.Errorf("original reconcile error"), + setupFake: func(fc *fakes.FakeCtrlClient) { + fc.StatusUpdateReturns(fmt.Errorf("status update conflict")) + }, + wantErr: true, + // BUG: The istiocsr updateCondition at utils.go line 484 passes `err` + // (the raw updateStatus error) instead of `prependErr` into the aggregate. + // This means the returned aggregate contains the status update error twice + // (once raw and once wrapped) but LOSES the original prependErr. + // + // The http01proxy version correctly uses `prependErr`: + // return utilerrors.NewAggregate([]error{prependErr, errUpdate}) + // + // The istiocsr version incorrectly uses `err`: + // return utilerrors.NewAggregate([]error{err, errUpdate}) + // + // This test documents the current (buggy) behavior. + wantErrMsg: "failed to update", + checkBuggyAggregate: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mock := &fakes.FakeCtrlClient{} + if tt.setupFake != nil { + tt.setupFake(mock) + } + r := testReconciler(t) + r.CtrlClient = mock + + istiocsr := testIstioCSR() + err := r.updateCondition(istiocsr, tt.prependErr) + + if tt.wantErr { + if err == nil { + t.Fatal("expected error, got nil") + } + if !strings.Contains(err.Error(), tt.wantErrMsg) { + t.Errorf("updateCondition() err = %q, want substring %q", err.Error(), tt.wantErrMsg) + } + + // Document the bug: when prependErr is non-nil and status update fails, + // the aggregate should contain prependErr but doesn't due to the bug. + if tt.checkBuggyAggregate { + errStr := err.Error() + // The correct behavior would include "original reconcile error" in the aggregate. + // Due to the bug (using `err` instead of `prependErr`), it does NOT. + if strings.Contains(errStr, "original reconcile error") { + // If this assertion fires, the bug has been fixed -- update this test. + t.Log("NOTE: updateCondition now correctly includes prependErr in aggregate -- bug is fixed") + } else { + t.Log("KNOWN BUG: updateCondition uses `err` instead of `prependErr` in aggregate (istiocsr utils.go line 484)") + } + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + } +} diff --git a/pkg/controller/trustmanager/deployments_test.go b/pkg/controller/trustmanager/deployments_test.go index 8e86c456d..7d2a6b7f2 100644 --- a/pkg/controller/trustmanager/deployments_test.go +++ b/pkg/controller/trustmanager/deployments_test.go @@ -421,6 +421,144 @@ func TestDeploymentOverrides(t *testing.T) { } } +func TestContainerPortsMatch(t *testing.T) { + tests := []struct { + name string + desired []corev1.ContainerPort + existing []corev1.ContainerPort + want bool + }{ + { + name: "identical ports match", + desired: []corev1.ContainerPort{{Name: "https", ContainerPort: 6443}}, + existing: []corev1.ContainerPort{{Name: "https", ContainerPort: 6443}}, + want: true, + }, + { + name: "mismatched port counts do not match", + desired: []corev1.ContainerPort{{Name: "https", ContainerPort: 6443}}, + existing: []corev1.ContainerPort{{Name: "https", ContainerPort: 6443}, {Name: "metrics", ContainerPort: 9402}}, + want: false, + }, + { + name: "same count but unmatched port name", + desired: []corev1.ContainerPort{{Name: "https", ContainerPort: 6443}}, + existing: []corev1.ContainerPort{{Name: "webhook", ContainerPort: 6443}}, + want: false, + }, + { + name: "same count but unmatched port number", + desired: []corev1.ContainerPort{{Name: "https", ContainerPort: 6443}}, + existing: []corev1.ContainerPort{{Name: "https", ContainerPort: 8443}}, + want: false, + }, + { + name: "both empty match", + desired: nil, + existing: nil, + want: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := containerPortsMatch(tt.desired, tt.existing) + if got != tt.want { + t.Errorf("containerPortsMatch() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestReadinessProbeModified(t *testing.T) { + tests := []struct { + name string + desired *corev1.Probe + existing *corev1.Probe + want bool + }{ + { + name: "both nil not modified", + desired: nil, + existing: nil, + want: false, + }, + { + name: "desired nil existing non-nil is modified", + desired: nil, + existing: &corev1.Probe{}, + want: true, + }, + { + name: "desired non-nil existing nil is modified", + desired: &corev1.Probe{}, + existing: nil, + want: true, + }, + { + name: "HTTPGet path difference is modified", + desired: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{Path: "/readyz"}, + }, + }, + existing: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{Path: "/healthz"}, + }, + }, + want: true, + }, + { + name: "identical HTTPGet probes not modified", + desired: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{Path: "/readyz"}, + }, + InitialDelaySeconds: 3, + PeriodSeconds: 7, + }, + existing: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{Path: "/readyz"}, + }, + InitialDelaySeconds: 3, + PeriodSeconds: 7, + }, + want: false, + }, + { + name: "desired has HTTPGet existing does not is modified", + desired: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{Path: "/readyz"}, + }, + }, + existing: &corev1.Probe{}, + want: true, + }, + { + name: "different PeriodSeconds is modified", + desired: &corev1.Probe{ + PeriodSeconds: 10, + }, + existing: &corev1.Probe{ + PeriodSeconds: 5, + }, + want: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := readinessProbeModified(tt.desired, tt.existing) + if got != tt.want { + t.Errorf("readinessProbeModified() = %v, want %v", got, tt.want) + } + }) + } +} + func TestDeploymentReconciliation(t *testing.T) { tests := []struct { name string diff --git a/pkg/controller/trustmanager/utils_test.go b/pkg/controller/trustmanager/utils_test.go index a113ace02..4e3f12583 100644 --- a/pkg/controller/trustmanager/utils_test.go +++ b/pkg/controller/trustmanager/utils_test.go @@ -1,6 +1,7 @@ package trustmanager import ( + "context" "fmt" "strings" "testing" @@ -8,9 +9,11 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" "github.com/openshift/cert-manager-operator/api/operator/v1alpha1" "github.com/openshift/cert-manager-operator/pkg/controller/common" + "github.com/openshift/cert-manager-operator/pkg/controller/common/fakes" ) func TestGetTrustNamespace(t *testing.T) { @@ -354,3 +357,171 @@ func TestDecodeServiceAccountObjBytes(t *testing.T) { }) } } + +func TestManagedAnnotationsModified(t *testing.T) { + tests := []struct { + name string + desiredAnnotations map[string]string + currentAnnotations map[string]string + wantModified bool + }{ + { + name: "identical annotations not modified", + desiredAnnotations: map[string]string{"key": "value"}, + currentAnnotations: map[string]string{"key": "value"}, + wantModified: false, + }, + { + name: "missing managed annotation is modified", + desiredAnnotations: map[string]string{"managed": "value"}, + currentAnnotations: map[string]string{"other": "value"}, + wantModified: true, + }, + { + name: "changed managed annotation is modified", + desiredAnnotations: map[string]string{"key": "desired"}, + currentAnnotations: map[string]string{"key": "tampered"}, + wantModified: true, + }, + { + name: "extra annotation on existing is allowed", + desiredAnnotations: map[string]string{"managed": "value"}, + currentAnnotations: map[string]string{"managed": "value", "extra": "ok"}, + wantModified: false, + }, + { + name: "nil desired annotations not modified", + desiredAnnotations: nil, + currentAnnotations: map[string]string{"any": "value"}, + wantModified: false, + }, + { + name: "nil existing annotations with desired is modified", + desiredAnnotations: map[string]string{"key": "value"}, + currentAnnotations: nil, + wantModified: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + desired := testTrustManager().Build() + desired.SetAnnotations(tt.desiredAnnotations) + + existing := testTrustManager().Build() + existing.SetAnnotations(tt.currentAnnotations) + + got := managedAnnotationsModified(desired, existing) + if got != tt.wantModified { + t.Errorf("expected modified=%v, got %v", tt.wantModified, got) + } + }) + } +} + +func TestAddFinalizerAlreadyPresent(t *testing.T) { + r := testReconciler(t) + mock := &fakes.FakeCtrlClient{} + r.CtrlClient = mock + + tm := testTrustManager().Build() + tm.Finalizers = []string{finalizer} + + err := r.addFinalizer(context.Background(), tm) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // When the finalizer is already present, no Update or Get calls should occur. + if got := mock.UpdateWithRetryCallCount(); got != 0 { + t.Errorf("expected 0 UpdateWithRetry calls, got %d", got) + } + if got := mock.GetCallCount(); got != 0 { + t.Errorf("expected 0 Get calls, got %d", got) + } +} + +func TestAddFinalizerGetAfterUpdateFails(t *testing.T) { + r := testReconciler(t) + mock := &fakes.FakeCtrlClient{} + // UpdateWithRetry succeeds, but Get after update fails. + mock.GetCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) error { + return errTestClient + }) + r.CtrlClient = mock + + tm := testTrustManager().Build() + + err := r.addFinalizer(context.Background(), tm) + if err == nil { + t.Fatal("expected error, got nil") + } + assertError(t, err, "failed to fetch trustmanager.openshift.operator.io") +} + +func TestRemoveFinalizerNotPresent(t *testing.T) { + r := testReconciler(t) + mock := &fakes.FakeCtrlClient{} + r.CtrlClient = mock + + tm := testTrustManager().Build() + // No finalizers set — removeFinalizer should be a no-op. + + err := r.removeFinalizer(context.Background(), tm, finalizer) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if got := mock.UpdateWithRetryCallCount(); got != 0 { + t.Errorf("expected 0 UpdateWithRetry calls, got %d", got) + } +} + +func TestUpdateStatusGetFailure(t *testing.T) { + r := testReconciler(t) + mock := &fakes.FakeCtrlClient{} + mock.GetCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) error { + return errTestClient + }) + r.CtrlClient = mock + + tm := testTrustManager().Build() + tm.Name = trustManagerObjectName + tm.Status.Conditions = []metav1.Condition{ + { + Type: v1alpha1.Ready, + Status: metav1.ConditionTrue, + }, + } + + err := r.updateStatus(context.Background(), tm) + if err == nil { + t.Fatal("expected error, got nil") + } + assertError(t, err, "failed to fetch trustmanager.openshift.operator.io") +} + +func TestUpdateStatusStatusUpdateFailure(t *testing.T) { + r := testReconciler(t) + mock := &fakes.FakeCtrlClient{} + mock.GetCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) error { + switch o := obj.(type) { + case *v1alpha1.TrustManager: + testTrustManager().Build().DeepCopyInto(o) + } + return nil + }) + mock.StatusUpdateCalls(func(ctx context.Context, obj client.Object, opts ...client.SubResourceUpdateOption) error { + return errTestClient + }) + r.CtrlClient = mock + + tm := testTrustManager().Build() + tm.Name = trustManagerObjectName + + err := r.updateStatus(context.Background(), tm) + if err == nil { + t.Fatal("expected error, got nil") + } + assertError(t, err, "failed to update trustmanager.openshift.operator.io") +} diff --git a/pkg/controller/trustmanager/webhooks_test.go b/pkg/controller/trustmanager/webhooks_test.go index 17841c685..c357bb550 100644 --- a/pkg/controller/trustmanager/webhooks_test.go +++ b/pkg/controller/trustmanager/webhooks_test.go @@ -197,6 +197,41 @@ func TestValidatingWebhookConfigReconciliation(t *testing.T) { wantExistsCount: 1, wantPatchCount: 1, }, + { + name: "apply when existing has rules drift", + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + vwc := getValidatingWebhookConfigObject(testResourceLabels(), testResourceAnnotations()) + vwc.Webhooks[0].Rules = []admissionregistrationv1.RuleWithOperations{ + { + Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.Create}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{"tampered.io"}, + APIVersions: []string{"v1"}, + Resources: []string{"fakes"}, + }, + }, + } + vwc.DeepCopyInto(obj.(*admissionregistrationv1.ValidatingWebhookConfiguration)) + return true, nil + }) + }, + wantExistsCount: 1, + wantPatchCount: 1, + }, + { + name: "apply when existing has admission review versions drift", + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + vwc := getValidatingWebhookConfigObject(testResourceLabels(), testResourceAnnotations()) + vwc.Webhooks[0].AdmissionReviewVersions = []string{"v1beta1"} + vwc.DeepCopyInto(obj.(*admissionregistrationv1.ValidatingWebhookConfiguration)) + return true, nil + }) + }, + wantExistsCount: 1, + wantPatchCount: 1, + }, { name: "exists error propagates", preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { diff --git a/pkg/features/features_test.go b/pkg/features/features_test.go index b064b67d0..0011e3321 100644 --- a/pkg/features/features_test.go +++ b/pkg/features/features_test.go @@ -293,3 +293,54 @@ func TestIsTrustManagerFeatureGateEnabled(t *testing.T) { }) } } + +// TestIsIstioCSRFeatureGateEnabled covers the IstioCSR operator featuregate +// (--unsupported-addon-features). IstioCSR is GA and enabled by default, +// so it does not depend on cluster FeatureSet (unlike TrustManager). +func TestIsIstioCSRFeatureGateEnabled(t *testing.T) { + defer func() { + // IstioCSR defaults to true; restore after test + _ = SetupWithFlagValue("IstioCSR=true") + }() + + tests := []struct { + name string + prep func(t *testing.T) + assert func(t *testing.T) + }{ + { + name: "returns true when operator featuregate is on", + prep: func(t *testing.T) { + t.Helper() + require.NoError(t, SetupWithFlagValue("IstioCSR=true")) + }, + assert: func(t *testing.T) { + t.Helper() + assert.True(t, IsIstioCSRFeatureGateEnabled()) + }, + }, + { + name: "returns false when operator featuregate is off", + prep: func(t *testing.T) { + t.Helper() + require.NoError(t, SetupWithFlagValue("IstioCSR=false")) + }, + assert: func(t *testing.T) { + t.Helper() + assert.False(t, IsIstioCSRFeatureGateEnabled()) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tt.prep(t) + tt.assert(t) + }) + } +} + +func TestSetupWithFlagValue_invalidFlag(t *testing.T) { + err := SetupWithFlagValue("InvalidFeature=true") + require.Error(t, err) +} diff --git a/pkg/operator/operatorclient/operatorclient_test.go b/pkg/operator/operatorclient/operatorclient_test.go new file mode 100644 index 000000000..7d16a004e --- /dev/null +++ b/pkg/operator/operatorclient/operatorclient_test.go @@ -0,0 +1,373 @@ +package operatorclient + +import ( + "context" + "encoding/json" + "fmt" + "testing" + + operatorv1 "github.com/openshift/api/operator/v1" + applyoperatorv1 "github.com/openshift/client-go/operator/applyconfigurations/operator/v1" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + ktesting "k8s.io/client-go/testing" + "k8s.io/utils/clock" + + "github.com/openshift/cert-manager-operator/api/operator/v1alpha1" + fakeclientset "github.com/openshift/cert-manager-operator/pkg/operator/clientset/versioned/fake" + informers "github.com/openshift/cert-manager-operator/pkg/operator/informers/externalversions" +) + +// newTestOperatorClient creates an OperatorClient backed by a fake clientset. +// The provided objects are seeded into the fake client and informer cache. +func newTestOperatorClient(t *testing.T, objects ...runtime.Object) (OperatorClient, *fakeclientset.Clientset) { + t.Helper() + + fakeClient := fakeclientset.NewClientset(objects...) + factory := informers.NewSharedInformerFactory(fakeClient, 0) + + // Pre-populate the informer cache with the objects so the lister works. + informer := factory.Operator().V1alpha1().CertManagers().Informer() + for _, obj := range objects { + if err := informer.GetIndexer().Add(obj); err != nil { + t.Fatalf("failed to add object to indexer: %v", err) + } + } + + oc := OperatorClient{ + Informers: factory, + Client: fakeClient.OperatorV1alpha1(), + Clock: clock.RealClock{}, + } + return oc, fakeClient +} + +// newCertManager creates a CertManager resource for testing. +func newCertManager(opts ...func(*v1alpha1.CertManager)) *v1alpha1.CertManager { + cm := &v1alpha1.CertManager{ + TypeMeta: metav1.TypeMeta{ + Kind: "CertManager", + APIVersion: "operator.openshift.io/v1alpha1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster", + ResourceVersion: "1", + }, + Spec: v1alpha1.CertManagerSpec{ + OperatorSpec: operatorv1.OperatorSpec{ + ManagementState: operatorv1.Managed, + }, + }, + } + for _, fn := range opts { + fn(cm) + } + return cm +} + +func TestGetUnsupportedConfigOverrides(t *testing.T) { + tests := []struct { + name string + rawBytes []byte + expectNil bool + expectErr bool + expectValue *v1alpha1.UnsupportedConfigOverrides + }{ + { + name: "empty raw bytes returns nil config", + rawBytes: nil, + expectNil: true, + }, + { + name: "valid JSON returns parsed UnsupportedConfigOverrides", + rawBytes: []byte(`{"controller":{"args":["--foo","--bar"]}}`), + expectValue: &v1alpha1.UnsupportedConfigOverrides{ + Controller: v1alpha1.UnsupportedConfigOverridesForCertManagerController{ + Args: []string{"--foo", "--bar"}, + }, + }, + }, + { + name: "invalid JSON returns unmarshal error", + rawBytes: []byte(`{not-valid-json`), + expectErr: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + spec := &operatorv1.OperatorSpec{ + UnsupportedConfigOverrides: runtime.RawExtension{ + Raw: tc.rawBytes, + }, + } + result, err := GetUnsupportedConfigOverrides(spec) + + if tc.expectErr { + if err == nil { + t.Fatal("expected error, got nil") + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if tc.expectNil { + if result != nil { + t.Fatalf("expected nil result, got %+v", result) + } + return + } + + got, _ := json.Marshal(result) + want, _ := json.Marshal(tc.expectValue) + if string(got) != string(want) { + t.Errorf("expected %s, got %s", want, got) + } + }) + } +} + +func TestGetOperatorState(t *testing.T) { + t.Run("success returns spec, status, and resource version", func(t *testing.T) { + cm := newCertManager(func(cm *v1alpha1.CertManager) { + cm.ResourceVersion = "42" + cm.Spec.ManagementState = operatorv1.Unmanaged + }) + oc, _ := newTestOperatorClient(t, cm) + + spec, status, rv, err := oc.GetOperatorState() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if rv != "42" { + t.Errorf("expected resource version 42, got %s", rv) + } + if spec.ManagementState != operatorv1.Unmanaged { + t.Errorf("expected ManagementState Unmanaged, got %v", spec.ManagementState) + } + if status == nil { + t.Fatal("expected non-nil status") + } + }) + + t.Run("get error propagates when resource not in lister", func(t *testing.T) { + oc, _ := newTestOperatorClient(t) + + _, _, _, err := oc.GetOperatorState() + if err == nil { + t.Fatal("expected error when no resource exists, got nil") + } + }) +} + +func TestEnsureFinalizer(t *testing.T) { + ctx := context.Background() + + t.Run("adds finalizer when not present", func(t *testing.T) { + cm := newCertManager() + oc, fakeClient := newTestOperatorClient(t, cm) + + if err := oc.EnsureFinalizer(ctx, "test-finalizer"); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + updated, err := fakeClient.OperatorV1alpha1().CertManagers().Get(ctx, "cluster", metav1.GetOptions{}) + if err != nil { + t.Fatalf("failed to get updated resource: %v", err) + } + found := false + for _, f := range updated.GetFinalizers() { + if f == "test-finalizer" { + found = true + break + } + } + if !found { + t.Error("expected finalizer 'test-finalizer' to be present on updated resource") + } + }) + + t.Run("no-op when finalizer already present", func(t *testing.T) { + cm := newCertManager(func(cm *v1alpha1.CertManager) { + cm.SetFinalizers([]string{"test-finalizer"}) + }) + oc, fakeClient := newTestOperatorClient(t, cm) + + if err := oc.EnsureFinalizer(ctx, "test-finalizer"); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + updated, err := fakeClient.OperatorV1alpha1().CertManagers().Get(ctx, "cluster", metav1.GetOptions{}) + if err != nil { + t.Fatalf("failed to get resource: %v", err) + } + if updated.ResourceVersion != "1" { + t.Errorf("expected no update (resource version 1), got %s", updated.ResourceVersion) + } + }) + + t.Run("save error propagates", func(t *testing.T) { + cm := newCertManager() + oc, fakeClient := newTestOperatorClient(t, cm) + + fakeClient.PrependReactor("update", "certmanagers", + func(action ktesting.Action) (bool, runtime.Object, error) { + return true, nil, fmt.Errorf("simulated update error") + }) + + err := oc.EnsureFinalizer(ctx, "new-finalizer") + if err == nil { + t.Fatal("expected error from save, got nil") + } + if err.Error() != "simulated update error" { + t.Errorf("unexpected error message: %v", err) + } + }) +} + +func TestRemoveFinalizer(t *testing.T) { + ctx := context.Background() + + t.Run("removes finalizer when present", func(t *testing.T) { + cm := newCertManager(func(cm *v1alpha1.CertManager) { + cm.SetFinalizers([]string{"keep-me", "remove-me", "also-keep"}) + }) + oc, fakeClient := newTestOperatorClient(t, cm) + + if err := oc.RemoveFinalizer(ctx, "remove-me"); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + updated, err := fakeClient.OperatorV1alpha1().CertManagers().Get(ctx, "cluster", metav1.GetOptions{}) + if err != nil { + t.Fatalf("failed to get updated resource: %v", err) + } + for _, f := range updated.GetFinalizers() { + if f == "remove-me" { + t.Error("finalizer 'remove-me' should have been removed") + } + } + if len(updated.GetFinalizers()) != 2 { + t.Errorf("expected 2 remaining finalizers, got %d", len(updated.GetFinalizers())) + } + }) + + t.Run("no-op when finalizer not present", func(t *testing.T) { + cm := newCertManager(func(cm *v1alpha1.CertManager) { + cm.SetFinalizers([]string{"other-finalizer"}) + }) + oc, fakeClient := newTestOperatorClient(t, cm) + + if err := oc.RemoveFinalizer(ctx, "not-present"); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + updated, err := fakeClient.OperatorV1alpha1().CertManagers().Get(ctx, "cluster", metav1.GetOptions{}) + if err != nil { + t.Fatalf("failed to get resource: %v", err) + } + if updated.ResourceVersion != "1" { + t.Errorf("expected no update (resource version 1), got %s", updated.ResourceVersion) + } + }) + + t.Run("save error propagates", func(t *testing.T) { + cm := newCertManager(func(cm *v1alpha1.CertManager) { + cm.SetFinalizers([]string{"test-finalizer"}) + }) + oc, fakeClient := newTestOperatorClient(t, cm) + + fakeClient.PrependReactor("update", "certmanagers", + func(action ktesting.Action) (bool, runtime.Object, error) { + return true, nil, fmt.Errorf("simulated update error") + }) + + err := oc.RemoveFinalizer(ctx, "test-finalizer") + if err == nil { + t.Fatal("expected error from save, got nil") + } + if err.Error() != "simulated update error" { + t.Errorf("unexpected error message: %v", err) + } + }) +} + +func TestApplyOperatorStatus(t *testing.T) { + ctx := context.Background() + + t.Run("nil desiredConfiguration returns error", func(t *testing.T) { + cm := newCertManager() + oc, _ := newTestOperatorClient(t, cm) + + err := oc.ApplyOperatorStatus(ctx, "test-manager", nil) + if err == nil { + t.Fatal("expected error for nil desiredConfiguration, got nil") + } + expected := "applyConfiguration must have a value" + if err.Error() != expected { + t.Errorf("expected error %q, got %q", expected, err.Error()) + } + }) + + t.Run("not-found error creates new status", func(t *testing.T) { + // No CertManager resource exists in the fake client. + oc, _ := newTestOperatorClient(t) + + desired := applyoperatorv1.OperatorStatus() + err := oc.ApplyOperatorStatus(ctx, "test-manager", desired) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + + t.Run("get error (non-NotFound) propagates", func(t *testing.T) { + cm := newCertManager() + oc, fakeClient := newTestOperatorClient(t, cm) + + fakeClient.PrependReactor("get", "certmanagers", + func(action ktesting.Action) (bool, runtime.Object, error) { + return true, nil, fmt.Errorf("simulated server error") + }) + + desired := applyoperatorv1.OperatorStatus() + err := oc.ApplyOperatorStatus(ctx, "test-manager", desired) + if err == nil { + t.Fatal("expected error, got nil") + } + if expected := "unable to get operator configuration: simulated server error"; err.Error() != expected { + t.Errorf("expected error %q, got %q", expected, err.Error()) + } + }) + + t.Run("deep-equal status skips update", func(t *testing.T) { + cm := newCertManager() + oc, fakeClient := newTestOperatorClient(t, cm) + + // Apply once to establish the baseline. + desired := applyoperatorv1.OperatorStatus() + if err := oc.ApplyOperatorStatus(ctx, "test-manager", desired); err != nil { + t.Fatalf("first apply failed: %v", err) + } + + // Track whether a second apply-status call is made. + applyStatusCalled := false + fakeClient.PrependReactor("patch", "certmanagers", + func(action ktesting.Action) (bool, runtime.Object, error) { + applyStatusCalled = true + return false, nil, nil + }) + + // Apply the same status again -- should be a no-op. + desired2 := applyoperatorv1.OperatorStatus() + if err := oc.ApplyOperatorStatus(ctx, "test-manager", desired2); err != nil { + t.Fatalf("second apply failed: %v", err) + } + + if applyStatusCalled { + t.Error("expected no apply-status call when status is unchanged") + } + }) +} diff --git a/pkg/operator/setup_manager_test.go b/pkg/operator/setup_manager_test.go new file mode 100644 index 000000000..4b7489d7f --- /dev/null +++ b/pkg/operator/setup_manager_test.go @@ -0,0 +1,232 @@ +package operator + +import ( + "testing" + + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" + "sigs.k8s.io/controller-runtime/pkg/cache" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/openshift/cert-manager-operator/pkg/controller/common" + "github.com/openshift/cert-manager-operator/pkg/controller/istiocsr" + "github.com/openshift/cert-manager-operator/pkg/controller/trustmanager" +) + +func TestBuildCacheObjectList(t *testing.T) { + t.Run("single controller enabled (IstioCSR)", func(t *testing.T) { + config := ControllerConfig{ + EnableIstioCSR: true, + EnableTrustManager: false, + } + + objectList, err := buildCacheObjectList(config) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // IstioCSR manages 9 resource types plus 1 CR type. + // Verify at least the Deployment entry exists with the correct label selector. + found := false + for key, byObj := range objectList { + if _, ok := key.(*appsv1.Deployment); ok { + found = true + sel := byObj.Label + if sel == nil { + t.Fatal("expected label selector for Deployment, got nil") + } + selectorStr := sel.String() + if selectorStr == "" { + t.Fatal("expected non-empty label selector for Deployment") + } + // Should match only the IstioCSR label value. + testLabels := map[string]string{common.ManagedResourceLabelKey: istiocsr.RequestEnqueueLabelValue} + if !sel.Matches(labelSet(testLabels)) { + t.Errorf("expected selector to match IstioCSR label value %q, selector: %s", + istiocsr.RequestEnqueueLabelValue, selectorStr) + } + break + } + } + if !found { + t.Error("expected Deployment entry in cache object list") + } + }) + + t.Run("multiple controllers enabled (labels merge)", func(t *testing.T) { + config := ControllerConfig{ + EnableIstioCSR: true, + EnableTrustManager: true, + } + + objectList, err := buildCacheObjectList(config) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Deployment is shared between IstioCSR and TrustManager. + // The label selector should match both values via the In operator. + for key, byObj := range objectList { + if _, ok := key.(*appsv1.Deployment); ok { + sel := byObj.Label + if sel == nil { + t.Fatal("expected label selector for Deployment, got nil") + } + + istioLabels := map[string]string{common.ManagedResourceLabelKey: istiocsr.RequestEnqueueLabelValue} + trustLabels := map[string]string{common.ManagedResourceLabelKey: trustmanager.RequestEnqueueLabelValue} + + if !sel.Matches(labelSet(istioLabels)) { + t.Errorf("expected merged selector to match IstioCSR label, selector: %s", sel.String()) + } + if !sel.Matches(labelSet(trustLabels)) { + t.Errorf("expected merged selector to match TrustManager label, selector: %s", sel.String()) + } + break + } + } + }) + + t.Run("no controllers enabled", func(t *testing.T) { + config := ControllerConfig{ + EnableIstioCSR: false, + EnableTrustManager: false, + } + + objectList, err := buildCacheObjectList(config) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(objectList) != 0 { + t.Errorf("expected empty object list when no controllers enabled, got %d entries", len(objectList)) + } + }) +} + +func TestAddControllerCacheConfig(t *testing.T) { + t.Run("adds new resource type", func(t *testing.T) { + objectList := make(map[client.Object]cache.ByObject) + resources := []client.Object{&appsv1.Deployment{}, &corev1.Service{}} + + err := addControllerCacheConfig(objectList, "test-controller", resources) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(objectList) != 2 { + t.Errorf("expected 2 entries, got %d", len(objectList)) + } + + // Verify the Deployment entry has the correct label selector. + _, byObj, found := findExistingCacheEntry(objectList, &appsv1.Deployment{}) + if !found { + t.Fatal("expected Deployment entry in object list") + } + testLabels := map[string]string{common.ManagedResourceLabelKey: "test-controller"} + if !byObj.Label.Matches(labelSet(testLabels)) { + t.Errorf("expected selector to match label value %q, selector: %s", "test-controller", byObj.Label.String()) + } + }) + + t.Run("merges existing resource type with In operator", func(t *testing.T) { + objectList := make(map[client.Object]cache.ByObject) + resources1 := []client.Object{&appsv1.Deployment{}} + resources2 := []client.Object{&appsv1.Deployment{}} + + err := addControllerCacheConfig(objectList, "controller-a", resources1) + if err != nil { + t.Fatalf("first addControllerCacheConfig failed: %v", err) + } + + err = addControllerCacheConfig(objectList, "controller-b", resources2) + if err != nil { + t.Fatalf("second addControllerCacheConfig failed: %v", err) + } + + // Should still be 1 entry (merged), not 2. + if len(objectList) != 1 { + t.Errorf("expected 1 merged entry, got %d", len(objectList)) + } + + _, byObj, found := findExistingCacheEntry(objectList, &appsv1.Deployment{}) + if !found { + t.Fatal("expected Deployment entry in object list") + } + + // Selector should match both label values. + labelsA := map[string]string{common.ManagedResourceLabelKey: "controller-a"} + labelsB := map[string]string{common.ManagedResourceLabelKey: "controller-b"} + if !byObj.Label.Matches(labelSet(labelsA)) { + t.Errorf("expected merged selector to match controller-a, selector: %s", byObj.Label.String()) + } + if !byObj.Label.Matches(labelSet(labelsB)) { + t.Errorf("expected merged selector to match controller-b, selector: %s", byObj.Label.String()) + } + + // Should NOT match a label value that was never added. + labelsC := map[string]string{common.ManagedResourceLabelKey: "controller-c"} + if byObj.Label.Matches(labelSet(labelsC)) { + t.Errorf("expected merged selector NOT to match controller-c, selector: %s", byObj.Label.String()) + } + }) +} + +func TestFindExistingCacheEntry(t *testing.T) { + t.Run("returns entry when type found", func(t *testing.T) { + objectList := map[client.Object]cache.ByObject{ + &appsv1.Deployment{}: {}, + &corev1.Service{}: {}, + &rbacv1.ClusterRole{}: {}, + } + + key, _, found := findExistingCacheEntry(objectList, &appsv1.Deployment{}) + if !found { + t.Fatal("expected to find Deployment entry") + } + if _, ok := key.(*appsv1.Deployment); !ok { + t.Errorf("expected key to be *appsv1.Deployment, got %T", key) + } + }) + + t.Run("returns not-found when type absent", func(t *testing.T) { + objectList := map[client.Object]cache.ByObject{ + &appsv1.Deployment{}: {}, + &corev1.Service{}: {}, + } + + _, _, found := findExistingCacheEntry(objectList, &rbacv1.ClusterRole{}) + if found { + t.Error("expected not-found for ClusterRole, but it was found") + } + }) + + t.Run("distinguishes different pointer types of same kind", func(t *testing.T) { + objectList := map[client.Object]cache.ByObject{ + &rbacv1.ClusterRole{}: {}, + } + + _, _, found := findExistingCacheEntry(objectList, &rbacv1.ClusterRoleBinding{}) + if found { + t.Error("expected not-found for ClusterRoleBinding when only ClusterRole exists") + } + }) +} + +// labelSet wraps a map to satisfy labels.Labels interface via k8s.io/apimachinery. +type labelSet map[string]string + +func (ls labelSet) Has(label string) bool { + _, ok := ls[label] + return ok +} + +func (ls labelSet) Get(label string) string { + return ls[label] +} + +func (ls labelSet) Lookup(label string) (string, bool) { + v, ok := ls[label] + return v, ok +}