From de8017a6cb4c55c57a559b78f93174b619c3eacc Mon Sep 17 00:00:00 2001 From: Todd Short Date: Mon, 20 Oct 2025 17:06:32 -0400 Subject: [PATCH] Add TLS config observer to update opcon/catd Add an apiserver TLS config observer. This examines TLS config and copies the values into the .spec.observedConfig field of the olms.operator.openshift.io/cluster resource. This is then used to apply configuration to the operator-controller and catalogd. The configuration is applied to those deployments via: * --tls-custom-ciphers=X,Y,Z * --tls-custom-version=N * --tls-profile=custom In addition: * Move the deployment update hooks (proxy, TLS) into their own files * Add unit tests * Changed Apply()s to Patch()s when modifying olms to avoid overwrites * Update fieldManagers to avoid overwrites Signed-off-by: Todd Short Co-Authored-By: Claude --- cmd/cluster-olm-operator/main.go | 9 +- .../0000_51_olm_02_operator_clusterrole.yaml | 1 + pkg/clients/clients.go | 310 ++++++++-- pkg/clients/clients_test.go | 563 ++++++++++++++++++ pkg/controller/builder.go | 62 +- pkg/controller/builder_test.go | 67 --- pkg/controller/observedconfig_hook.go | 141 +++++ pkg/controller/observedconfig_hook_test.go | 205 +++++++ pkg/controller/proxy_hook.go | 81 +++ pkg/controller/proxy_hook_test.go | 75 +++ pkg/controller/proxycontroller.go | 6 - pkg/controller/tlsobserver.go | 166 ++++++ pkg/controller/tlsobserver_test.go | 110 ++++ .../operator/configobserver/apiserver/OWNERS | 8 + .../configobserver/apiserver/listers.go | 9 + .../configobserver/apiserver/observe_audit.go | 88 +++ .../configobserver/apiserver/observe_cors.go | 75 +++ .../apiserver/observe_tlssecurityprofile.go | 109 ++++ vendor/modules.txt | 1 + 19 files changed, 1909 insertions(+), 177 deletions(-) create mode 100644 pkg/clients/clients_test.go create mode 100644 pkg/controller/observedconfig_hook.go create mode 100644 pkg/controller/observedconfig_hook_test.go create mode 100644 pkg/controller/proxy_hook.go create mode 100644 pkg/controller/proxy_hook_test.go create mode 100644 pkg/controller/tlsobserver.go create mode 100644 pkg/controller/tlsobserver_test.go create mode 100644 vendor/github.com/openshift/library-go/pkg/operator/configobserver/apiserver/OWNERS create mode 100644 vendor/github.com/openshift/library-go/pkg/operator/configobserver/apiserver/listers.go create mode 100644 vendor/github.com/openshift/library-go/pkg/operator/configobserver/apiserver/observe_audit.go create mode 100644 vendor/github.com/openshift/library-go/pkg/operator/configobserver/apiserver/observe_cors.go create mode 100644 vendor/github.com/openshift/library-go/pkg/operator/configobserver/apiserver/observe_tlssecurityprofile.go diff --git a/cmd/cluster-olm-operator/main.go b/cmd/cluster-olm-operator/main.go index 5e967c21c..481ed9c56 100644 --- a/cmd/cluster-olm-operator/main.go +++ b/cmd/cluster-olm-operator/main.go @@ -182,6 +182,13 @@ func runOperator(ctx context.Context, cc *controllercmd.ControllerContext) error cc.EventRecorder.ForComponent(olmProxyController), ) + tlsObserverController := controller.NewTLSObserverController( + "OLMTLSSecurityProfileObserver", + cl.OperatorClient, + cl.ConfigInformerFactory, + cc.EventRecorder.ForComponent("OLMTLSSecurityProfileObserver"), + ) + versionGetter := status.NewVersionGetter() versionGetter.SetVersion("operator", status.VersionForOperatorFromEnv()) @@ -216,7 +223,7 @@ func runOperator(ctx context.Context, cc *controllercmd.ControllerContext) error return errors.New("timed out waiting for FeatureGate detection") } - for _, c := range append(staticResourceControllerList, upgradeableConditionController, incompatibleOperatorController, clusterOperatorController, operatorLoggingController, proxyController) { + for _, c := range append(staticResourceControllerList, upgradeableConditionController, incompatibleOperatorController, clusterOperatorController, operatorLoggingController, proxyController, tlsObserverController.Controller) { go func(c factory.Controller) { defer runtime.HandleCrash() c.Run(ctx, 1) diff --git a/manifests/0000_51_olm_02_operator_clusterrole.yaml b/manifests/0000_51_olm_02_operator_clusterrole.yaml index 37668582e..e8b19a14a 100644 --- a/manifests/0000_51_olm_02_operator_clusterrole.yaml +++ b/manifests/0000_51_olm_02_operator_clusterrole.yaml @@ -27,6 +27,7 @@ rules: - proxies - featuregates - clusterversions + - apiservers verbs: - get - list diff --git a/pkg/clients/clients.go b/pkg/clients/clients.go index c65620811..7d9a4dafb 100644 --- a/pkg/clients/clients.go +++ b/pkg/clients/clients.go @@ -10,6 +10,7 @@ import ( "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/klog/v2" "k8s.io/utils/clock" configv1 "github.com/openshift/api/config/v1" @@ -189,8 +190,10 @@ func setupFeatureGatesAccessor( var _ v1helpers.OperatorClientWithFinalizers = &OperatorClient{} const ( - globalConfigName = "cluster" - fieldManager = "cluster-olm-operator" + globalConfigName = "cluster" + fieldManager = "cluster-olm-operator" + specFieldManager = "cluster-olm-operator-spec" + finalizerFieldManager = "cluster-olm-operator-finalizer" ) type ClusterExtensionClient struct { @@ -280,36 +283,40 @@ func (o OperatorClient) ApplyOperatorSpec(ctx context.Context, fieldManager stri return fmt.Errorf("applyConfiguration must have a value") } - desiredSpec := &operatorv1apply.OLMSpecApplyConfiguration{ - OperatorSpecApplyConfiguration: *applyConfiguration, + // Convert apply configuration to OperatorSpec for patch generation + operatorSpec, err := convertApplyConfigToOperatorSpec(applyConfiguration) + if err != nil { + return fmt.Errorf("error converting apply configuration: %w", err) } - desired := operatorv1apply.OLM(globalConfigName) - desired.WithSpec(desiredSpec) instance, err := o.informers.Operator().V1().OLMs().Lister().Get(globalConfigName) - switch { - case apierrors.IsNotFound(err): - // do nothing and proceed with the apply - case err != nil: + if err != nil { return fmt.Errorf("unable to get operator configuration: %w", err) - default: - original, err := operatorv1apply.ExtractOLM(instance, fieldManager) - if err != nil { - return fmt.Errorf("unable to extract operator configuration: %w", err) - } - if equality.Semantic.DeepEqual(original, desired) { - return nil - } } - _, err = o.clientset.OperatorV1().OLMs().Apply(ctx, desired, metav1.ApplyOptions{ - Force: true, - FieldManager: fieldManager, - }) + // Check if changes are needed by comparing current spec + if needsUpdate, err := operatorSpecNeedsUpdate(instance.Spec.OperatorSpec, *operatorSpec); err != nil { + return fmt.Errorf("error checking if update needed: %w", err) + } else if !needsUpdate { + klog.V(2).Info("ApplyOperatorSpec: no changes detected, skipping patch operation") + return nil + } + + klog.V(2).Infof("ApplyOperatorSpec: changes detected, applying patch to %s", globalConfigName) + + // Generate patch using the same method as UpdateOperatorSpec + patch, err := generateOperatorSpecPatch(instance.ResourceVersion, operatorSpec) if err != nil { - return fmt.Errorf("unable to Apply for operator using fieldManager %q: %w", fieldManager, err) + return fmt.Errorf("error generating OperatorSpec patch: %w", err) } + _, err = o.clientset.OperatorV1().OLMs().Patch(ctx, globalConfigName, types.ApplyPatchType, patch, metav1.PatchOptions{FieldManager: finalizerFieldManager, Force: ptr.To(true)}) + if err != nil { + return fmt.Errorf("unable to patch operator spec using fieldManager %q: %w", fieldManager, err) + } + + klog.V(1).Infof("ApplyOperatorSpec: successfully applied operator spec patch to %s", globalConfigName) + return nil } @@ -426,14 +433,24 @@ func toStatusObj(in *operatorv1apply.OLMStatusApplyConfiguration) (*operatorv1.O } func (o OperatorClient) PatchOperatorStatus(ctx context.Context, jsonPatch *jsonpatch.PatchSet) error { + if jsonPatch == nil { + return fmt.Errorf("jsonPatch must have a value") + } + jsonPatchBytes, err := jsonPatch.Marshal() if err != nil { - return err + return fmt.Errorf("error marshaling JSON patch: %w", err) } - _, err = o.clientset.OperatorV1().OLMs().Patch(ctx, globalConfigName, types.JSONPatchType, jsonPatchBytes, metav1.PatchOptions{}, "/status") + + klog.V(2).Infof("PatchOperatorStatus: applying %d-byte JSON patch to %s status", len(jsonPatchBytes), globalConfigName) + + _, err = o.clientset.OperatorV1().OLMs().Patch(ctx, globalConfigName, types.JSONPatchType, jsonPatchBytes, metav1.PatchOptions{FieldManager: fieldManager, Force: ptr.To(true)}, "/status") if err != nil { return fmt.Errorf("unable to PatchOperatorStatus for operator using fieldManager %q: %w", fieldManager, err) } + + klog.V(1).Infof("PatchOperatorStatus: successfully patched %s status", globalConfigName) + return nil } @@ -466,12 +483,12 @@ func (o OperatorClient) GetOperatorStateWithQuorum(ctx context.Context) (*operat } func (o OperatorClient) UpdateOperatorSpec(ctx context.Context, oldResourceVersion string, in *operatorv1.OperatorSpec) (*operatorv1.OperatorSpec, string, error) { - patch, err := generateOLMPatch(oldResourceVersion, in, "spec") + patch, err := generateOperatorSpecPatch(oldResourceVersion, in) if err != nil { return nil, "", fmt.Errorf("error generating patch: %w", err) } - out, err := o.clientset.OperatorV1().OLMs().Patch(ctx, globalConfigName, types.ApplyPatchType, patch, metav1.PatchOptions{FieldManager: fieldManager, Force: ptr.To(true)}) + out, err := o.clientset.OperatorV1().OLMs().Patch(ctx, globalConfigName, types.ApplyPatchType, patch, metav1.PatchOptions{FieldManager: specFieldManager, Force: ptr.To(true)}) if err != nil { return nil, "", err } @@ -492,43 +509,134 @@ func (o OperatorClient) UpdateOperatorStatus(ctx context.Context, oldResourceVer } func (o OperatorClient) EnsureFinalizer(ctx context.Context, finalizer string) error { + if finalizer == "" { + return fmt.Errorf("finalizer must not be empty") + } + instance, err := o.informers.Operator().V1().OLMs().Lister().Get(globalConfigName) if err != nil { - return err + return fmt.Errorf("unable to get operator configuration: %w", err) + } + + currentFinalizers := instance.GetFinalizers() + + // Check if finalizer already exists + finalizersSet := sets.New(currentFinalizers...) + if finalizersSet.Has(finalizer) { + klog.V(2).Infof("EnsureFinalizer: finalizer %s already exists", finalizer) + return nil } - newFinalizers := sets.List(sets.New(instance.GetFinalizers()...).Insert(finalizer)) - olm := operatorv1apply.OLM(globalConfigName).WithFinalizers(newFinalizers...) - patch, err := json.Marshal(olm) + newFinalizers := sets.List(finalizersSet.Insert(finalizer)) + + patch, err := generateFinalizerPatch(instance.ResourceVersion, newFinalizers) if err != nil { - return err + return fmt.Errorf("error generating finalizer patch: %w", err) } - if _, err := o.clientset.OperatorV1().OLMs().Patch(ctx, globalConfigName, types.ApplyPatchType, patch, metav1.PatchOptions{FieldManager: fieldManager, Force: ptr.To(true)}); err != nil { - return err + klog.V(2).Infof("EnsureFinalizer: adding finalizer %s to %s", finalizer, globalConfigName) + + _, err = o.clientset.OperatorV1().OLMs().Patch(ctx, globalConfigName, types.ApplyPatchType, patch, metav1.PatchOptions{FieldManager: finalizerFieldManager, Force: ptr.To(true)}) + if err != nil { + return fmt.Errorf("unable to patch operator for finalizer %q: %w", finalizer, err) } + + klog.V(1).Infof("EnsureFinalizer: successfully added finalizer %s to %s", finalizer, globalConfigName) + return nil } func (o OperatorClient) RemoveFinalizer(ctx context.Context, finalizer string) error { + if finalizer == "" { + return fmt.Errorf("finalizer must not be empty") + } + instance, err := o.informers.Operator().V1().OLMs().Lister().Get(globalConfigName) if err != nil { - return err + return fmt.Errorf("unable to get operator configuration: %w", err) } - newFinalizers := sets.List(sets.New(instance.GetFinalizers()...).Delete(finalizer)) - olm := operatorv1apply.OLM(globalConfigName).WithFinalizers(newFinalizers...) - patch, err := json.Marshal(olm) + currentFinalizers := instance.GetFinalizers() + + // Check if finalizer exists + finalizersSet := sets.New(currentFinalizers...) + if !finalizersSet.Has(finalizer) { + klog.V(2).Infof("RemoveFinalizer: finalizer %s not found", finalizer) + return nil + } + + newFinalizers := sets.List(finalizersSet.Delete(finalizer)) + + patch, err := generateFinalizerPatch(instance.ResourceVersion, newFinalizers) if err != nil { - return err + return fmt.Errorf("error generating finalizer patch: %w", err) } - if _, err := o.clientset.OperatorV1().OLMs().Patch(ctx, globalConfigName, types.ApplyPatchType, patch, metav1.PatchOptions{FieldManager: fieldManager, Force: ptr.To(true)}); err != nil { - return err + klog.V(2).Infof("RemoveFinalizer: removing finalizer %s from %s", finalizer, globalConfigName) + + _, err = o.clientset.OperatorV1().OLMs().Patch(ctx, globalConfigName, types.ApplyPatchType, patch, metav1.PatchOptions{FieldManager: finalizerFieldManager, Force: ptr.To(true)}) + if err != nil { + return fmt.Errorf("unable to patch operator for finalizer %q removal: %w", finalizer, err) } + + klog.V(1).Infof("RemoveFinalizer: successfully removed finalizer %s from %s", finalizer, globalConfigName) + return nil } +func generateOperatorSpecPatch(_ string, operatorSpec *operatorv1.OperatorSpec) ([]byte, error) { + var u unstructured.Unstructured + u.SetAPIVersion(schema.GroupVersion{Group: operatorv1.GroupName, Version: "v1"}.String()) + u.SetKind("OLM") + // NOTE: Do NOT set resourceVersion in metadata to avoid conflicting with finalizer patches + // that also modify metadata fields using the same field manager + + // Convert the OperatorSpec to unstructured to properly handle RawExtension fields + specUnstructured, err := runtime.DefaultUnstructuredConverter.ToUnstructured(operatorSpec) + if err != nil { + return nil, fmt.Errorf("error converting OperatorSpec to unstructured: %w", err) + } + + // Create the spec object in the patch with all OperatorSpec fields + specObj := map[string]interface{}{ + "managementState": string(operatorSpec.ManagementState), + "logLevel": string(operatorSpec.LogLevel), + "operatorLogLevel": string(operatorSpec.OperatorLogLevel), + } + + // Handle RawExtension fields properly + if unsupportedConfig, found := specUnstructured["unsupportedConfigOverrides"]; found { + specObj["unsupportedConfigOverrides"] = unsupportedConfig + } else { + specObj["unsupportedConfigOverrides"] = nil + } + + if observedConfig, found := specUnstructured["observedConfig"]; found { + specObj["observedConfig"] = observedConfig + if klog.V(3).Enabled() { + if configBytes, err := json.Marshal(observedConfig); err == nil { + klog.V(3).Infof("OperatorSpec patch: observedConfig content: %s", string(configBytes)) + } + } + } else { + specObj["observedConfig"] = nil + } + + // Set the spec field with only OperatorSpec fields + if err := unstructured.SetNestedField(u.Object, specObj, "spec"); err != nil { + return nil, fmt.Errorf("error setting spec: %w", err) + } + + patchBytes, err := json.Marshal(u.Object) + if err != nil { + return nil, fmt.Errorf("error marshaling patch: %w", err) + } + + klog.V(2).Infof("OperatorSpec patch: generated %d-byte patch for operator spec", len(patchBytes)) + + return patchBytes, nil +} + func generateOLMPatch(resourceVersion string, in any, fieldPath ...string) ([]byte, error) { var u unstructured.Unstructured u.SetAPIVersion(schema.GroupVersion{Group: operatorv1.GroupName, Version: "v1"}.String()) @@ -540,9 +648,125 @@ func generateOLMPatch(resourceVersion string, in any, fieldPath ...string) ([]by return nil, fmt.Errorf("error converting to unstructured: %w", err) } + fieldPathStr := strings.Join(fieldPath, ".") if err := unstructured.SetNestedField(u.Object, inUnstructured, fieldPath...); err != nil { - return nil, fmt.Errorf("error setting %q: %w", strings.Join(fieldPath, "."), err) + return nil, fmt.Errorf("error setting %q: %w", fieldPathStr, err) + } + + patchBytes, err := json.Marshal(u.Object) + if err != nil { + return nil, fmt.Errorf("error marshaling patch: %w", err) + } + + klog.V(2).Infof("OLM patch: generated %d-byte patch for field path '%s'", len(patchBytes), fieldPathStr) + + return patchBytes, nil +} + +// generateFinalizerPatch creates a patch that only modifies the finalizers field +// without affecting other fields that might be managed by the same field manager +func generateFinalizerPatch(resourceVersion string, finalizers []string) ([]byte, error) { + var u unstructured.Unstructured + u.SetAPIVersion(schema.GroupVersion{Group: operatorv1.GroupName, Version: "v1"}.String()) + u.SetKind("OLM") + u.SetResourceVersion(resourceVersion) + + // Set only the finalizers field in metadata + if err := unstructured.SetNestedStringSlice(u.Object, finalizers, "metadata", "finalizers"); err != nil { + return nil, fmt.Errorf("error setting finalizers: %w", err) + } + + patchBytes, err := json.Marshal(u.Object) + if err != nil { + return nil, fmt.Errorf("error marshaling finalizer patch: %w", err) + } + + klog.V(2).Infof("Finalizer patch: generated %d-byte patch with %d finalizers", len(patchBytes), len(finalizers)) + + return patchBytes, nil +} + +// convertApplyConfigToOperatorSpec converts an OperatorSpecApplyConfiguration to an OperatorSpec +func convertApplyConfigToOperatorSpec(applyConfig *operatorv1apply.OperatorSpecApplyConfiguration) (*operatorv1.OperatorSpec, error) { + // Marshal apply configuration to JSON + jsonBytes, err := json.Marshal(applyConfig) + if err != nil { + return nil, fmt.Errorf("error marshaling apply configuration: %w", err) + } + + // Unmarshal to OperatorSpec + var operatorSpec operatorv1.OperatorSpec + if err := json.Unmarshal(jsonBytes, &operatorSpec); err != nil { + return nil, fmt.Errorf("error unmarshaling to OperatorSpec: %w", err) + } + + return &operatorSpec, nil +} + +// operatorSpecNeedsUpdate checks if the current OperatorSpec differs from the desired OperatorSpec +func operatorSpecNeedsUpdate(current, desired operatorv1.OperatorSpec) (bool, error) { + // Compare management state + if current.ManagementState != desired.ManagementState { + klog.V(2).Infof("ManagementState differs: current=%s, desired=%s", current.ManagementState, desired.ManagementState) + return true, nil + } + + // Compare log levels + if current.LogLevel != desired.LogLevel { + klog.V(2).Infof("LogLevel differs: current=%s, desired=%s", current.LogLevel, desired.LogLevel) + return true, nil + } + + if current.OperatorLogLevel != desired.OperatorLogLevel { + klog.V(2).Infof("OperatorLogLevel differs: current=%s, desired=%s", current.OperatorLogLevel, desired.OperatorLogLevel) + return true, nil + } + + // Compare RawExtension fields by converting to unstructured for comparison + currentUnsupportedConfig, err := rawExtensionToUnstructured(current.UnsupportedConfigOverrides) + if err != nil { + return false, fmt.Errorf("error converting current unsupportedConfigOverrides: %w", err) + } + + desiredUnsupportedConfig, err := rawExtensionToUnstructured(desired.UnsupportedConfigOverrides) + if err != nil { + return false, fmt.Errorf("error converting desired unsupportedConfigOverrides: %w", err) + } + + if !equality.Semantic.DeepEqual(currentUnsupportedConfig, desiredUnsupportedConfig) { + klog.V(2).Info("UnsupportedConfigOverrides differs") + return true, nil + } + + // Compare observedConfig + currentObservedConfig, err := rawExtensionToUnstructured(current.ObservedConfig) + if err != nil { + return false, fmt.Errorf("error converting current observedConfig: %w", err) + } + + desiredObservedConfig, err := rawExtensionToUnstructured(desired.ObservedConfig) + if err != nil { + return false, fmt.Errorf("error converting desired observedConfig: %w", err) + } + + if !equality.Semantic.DeepEqual(currentObservedConfig, desiredObservedConfig) { + klog.V(2).Info("ObservedConfig differs") + return true, nil + } + + return false, nil +} + +// rawExtensionToUnstructured converts a RawExtension to unstructured data for comparison +func rawExtensionToUnstructured(raw runtime.RawExtension) (interface{}, error) { + if len(raw.Raw) == 0 { + return nil, nil + } + + var result interface{} + if err := json.Unmarshal(raw.Raw, &result); err != nil { + return nil, fmt.Errorf("error unmarshaling RawExtension: %w", err) } - return json.Marshal(u.Object) + return result, nil } diff --git a/pkg/clients/clients_test.go b/pkg/clients/clients_test.go new file mode 100644 index 000000000..090b66e25 --- /dev/null +++ b/pkg/clients/clients_test.go @@ -0,0 +1,563 @@ +package clients + +import ( + "context" + "encoding/json" + "strings" + "testing" + + operatorv1 "github.com/openshift/api/operator/v1" + operatorv1apply "github.com/openshift/client-go/operator/applyconfigurations/operator/v1" + "k8s.io/apimachinery/pkg/runtime" +) + +func TestGenerateOperatorSpecPatch(t *testing.T) { + // Create a test OperatorSpec with all fields populated + testSpec := &operatorv1.OperatorSpec{ + ManagementState: operatorv1.Managed, + LogLevel: operatorv1.Debug, + OperatorLogLevel: operatorv1.Normal, + UnsupportedConfigOverrides: runtime.RawExtension{Raw: []byte(`{"test":"value"}`)}, + ObservedConfig: runtime.RawExtension{Raw: []byte(`{"olmTLSSecurityProfile":{"minTLSVersion":"VersionTLS12","cipherSuites":["TLS_AES_128_GCM_SHA256"]}}`)}, + } + + // Generate the patch + patchBytes, err := generateOperatorSpecPatch("test-version", testSpec) + if err != nil { + t.Fatalf("Error generating patch: %v", err) + } + + // Unmarshal the patch to verify its structure + var patchObj map[string]interface{} + if err := json.Unmarshal(patchBytes, &patchObj); err != nil { + t.Fatalf("Error unmarshaling patch: %v", err) + } + + // Verify top-level patch structure + if apiVersion, ok := patchObj["apiVersion"].(string); !ok || apiVersion != "operator.openshift.io/v1" { + t.Errorf("Expected apiVersion 'operator.openshift.io/v1', got %v", patchObj["apiVersion"]) + } + + if kind, ok := patchObj["kind"].(string); !ok || kind != "OLM" { + t.Errorf("Expected kind 'OLM', got %v", patchObj["kind"]) + } + + // Verify that metadata field is NOT present (to avoid conflicts with finalizer patches) + if _, hasMetadata := patchObj["metadata"]; hasMetadata { + t.Error("OperatorSpec patch should not contain metadata field to avoid conflicting with finalizer patches") + } + + // Verify spec field exists + spec, ok := patchObj["spec"].(map[string]interface{}) + if !ok { + t.Fatal("No spec field in patch") + } + + // Verify all OperatorSpec fields are present and correct + expectedFields := map[string]interface{}{ + "managementState": string(operatorv1.Managed), + "logLevel": string(operatorv1.Debug), + "operatorLogLevel": string(operatorv1.Normal), + } + + for field, expectedValue := range expectedFields { + if actualValue, exists := spec[field]; !exists { + t.Errorf("Expected field %s missing from patch", field) + } else if actualValue != expectedValue { + t.Errorf("Field %s: expected %v, got %v", field, expectedValue, actualValue) + } + } + + // Verify complex fields exist (we don't need to verify their exact content, just presence) + complexFields := []string{"unsupportedConfigOverrides", "observedConfig"} + for _, field := range complexFields { + if _, exists := spec[field]; !exists { + t.Errorf("Expected field %s missing from patch", field) + } + } + + // Verify that the patch only contains expected OperatorSpec fields + expectedFieldCount := 5 // managementState, logLevel, operatorLogLevel, unsupportedConfigOverrides, observedConfig + if len(spec) != expectedFieldCount { + t.Errorf("Expected %d fields in spec, got %d. Fields: %v", expectedFieldCount, len(spec), getKeys(spec)) + } + + // Verify no extra fields that shouldn't be there + for field := range spec { + switch field { + case "managementState", "logLevel", "operatorLogLevel", "unsupportedConfigOverrides", "observedConfig": + // These are expected OperatorSpec fields + default: + t.Errorf("Unexpected field %s in patch spec", field) + } + } +} + +func TestGenerateOperatorSpecPatchWithMinimalSpec(t *testing.T) { + // Test with minimal OperatorSpec (only required fields) + testSpec := &operatorv1.OperatorSpec{ + ManagementState: operatorv1.Managed, + // Other fields are optional and may be empty + } + + patchBytes, err := generateOperatorSpecPatch("minimal-version", testSpec) + if err != nil { + t.Fatalf("Error generating patch: %v", err) + } + + var patchObj map[string]interface{} + if err := json.Unmarshal(patchBytes, &patchObj); err != nil { + t.Fatalf("Error unmarshaling patch: %v", err) + } + + spec, ok := patchObj["spec"].(map[string]interface{}) + if !ok { + t.Fatal("No spec field in patch") + } + + // ManagementState should always be present + if managementState, exists := spec["managementState"]; !exists { + t.Error("managementState field missing from patch") + } else if managementState != string(operatorv1.Managed) { + t.Errorf("managementState: expected %s, got %v", operatorv1.Managed, managementState) + } + + // All OperatorSpec fields should be present (even if empty/nil) + expectedFields := []string{"managementState", "logLevel", "operatorLogLevel", "unsupportedConfigOverrides", "observedConfig"} + for _, field := range expectedFields { + if _, exists := spec[field]; !exists { + t.Errorf("Expected field %s missing from patch", field) + } + } +} + +func TestGenerateOperatorSpecPatchPreservesOLMFields(t *testing.T) { + // This test verifies that the patch function only includes OperatorSpec fields + // and would not overwrite hypothetical OLM-specific fields + + testSpec := &operatorv1.OperatorSpec{ + ManagementState: operatorv1.Managed, + ObservedConfig: runtime.RawExtension{Raw: []byte(`{"olmTLSSecurityProfile":{"minTLSVersion":"VersionTLS12"}}`)}, + } + + patchBytes, err := generateOperatorSpecPatch("preserve-test", testSpec) + if err != nil { + t.Fatalf("Error generating patch: %v", err) + } + + var patchObj map[string]interface{} + if err := json.Unmarshal(patchBytes, &patchObj); err != nil { + t.Fatalf("Error unmarshaling patch: %v", err) + } + + // Verify that the patch only targets the "spec" field at the top level + // This ensures it won't overwrite other OLM-specific fields that might exist + expectedTopLevelFields := []string{"apiVersion", "kind", "spec"} + for _, field := range expectedTopLevelFields { + if _, exists := patchObj[field]; !exists { + t.Errorf("Expected top-level field %s missing from patch", field) + } + } + + // Verify no unexpected top-level fields (especially no metadata to avoid conflicts) + for field := range patchObj { + found := false + for _, expected := range expectedTopLevelFields { + if field == expected { + found = true + break + } + } + if !found { + t.Errorf("Unexpected top-level field %s in patch", field) + } + } + + // Specifically verify metadata is NOT present (to avoid conflicting with finalizer patches) + if _, hasMetadata := patchObj["metadata"]; hasMetadata { + t.Error("OperatorSpec patch must not contain metadata field to avoid conflicting with finalizer patches") + } +} + +func TestGenerateOLMPatch(t *testing.T) { + // Test generateOLMPatch with status data + testStatus := &operatorv1.OperatorStatus{ + Conditions: []operatorv1.OperatorCondition{ + { + Type: "Available", + Status: operatorv1.ConditionTrue, + Reason: "AsExpected", + Message: "TLS observer is working correctly", + }, + }, + Version: "4.17.0", + } + + patchBytes, err := generateOLMPatch("status-test-version", testStatus, "status") + if err != nil { + t.Fatalf("generateOLMPatch failed: %v", err) + } + + // Verify the patch structure + var patchObj map[string]interface{} + if err := json.Unmarshal(patchBytes, &patchObj); err != nil { + t.Fatalf("Failed to unmarshal patch: %v", err) + } + + // Verify top-level structure + if apiVersion, ok := patchObj["apiVersion"].(string); !ok || apiVersion != "operator.openshift.io/v1" { + t.Errorf("Expected apiVersion 'operator.openshift.io/v1', got %v", patchObj["apiVersion"]) + } + + if kind, ok := patchObj["kind"].(string); !ok || kind != "OLM" { + t.Errorf("Expected kind 'OLM', got %v", patchObj["kind"]) + } + + // Verify status field exists + status, ok := patchObj["status"].(map[string]interface{}) + if !ok { + t.Fatal("No status field in patch") + } + + // Verify conditions exist + conditions, ok := status["conditions"].([]interface{}) + if !ok { + t.Fatal("No conditions field in status") + } + + if len(conditions) != 1 { + t.Errorf("Expected 1 condition, got %d", len(conditions)) + } + + // Verify condition content + condition := conditions[0].(map[string]interface{}) + if condition["type"] != "Available" { + t.Errorf("Expected condition type 'Available', got %v", condition["type"]) + } + if condition["status"] != "True" { + t.Errorf("Expected condition status 'True', got %v", condition["status"]) + } +} + +func TestGenerateOLMPatchWithObservedConfig(t *testing.T) { + // Test generateOLMPatch with observedConfig-like data (simulating what config observers would do) + testConfig := map[string]interface{}{ + "olmTLSSecurityProfile": map[string]interface{}{ + "minTLSVersion": "VersionTLS12", + "cipherSuites": []string{"TLS_AES_128_GCM_SHA256", "TLS_AES_256_GCM_SHA384"}, + }, + } + + patchBytes, err := generateOLMPatch("config-test-version", testConfig, "spec", "observedConfig") + if err != nil { + t.Fatalf("generateOLMPatch failed: %v", err) + } + + // Verify the patch structure + var patchObj map[string]interface{} + if err := json.Unmarshal(patchBytes, &patchObj); err != nil { + t.Fatalf("Failed to unmarshal patch: %v", err) + } + + // Navigate to nested observedConfig + spec, ok := patchObj["spec"].(map[string]interface{}) + if !ok { + t.Fatal("No spec field in patch") + } + + observedConfig, ok := spec["observedConfig"].(map[string]interface{}) + if !ok { + t.Fatal("No observedConfig field in spec") + } + + // Verify TLS profile + tlsProfile, ok := observedConfig["olmTLSSecurityProfile"].(map[string]interface{}) + if !ok { + t.Fatal("No olmTLSSecurityProfile in observedConfig") + } + + if tlsProfile["minTLSVersion"] != "VersionTLS12" { + t.Errorf("Expected minTLSVersion 'VersionTLS12', got %v", tlsProfile["minTLSVersion"]) + } + + cipherSuites, ok := tlsProfile["cipherSuites"].([]interface{}) + if !ok || len(cipherSuites) != 2 { + t.Errorf("Expected 2 cipher suites, got %v", cipherSuites) + } +} + +func TestGenerateOLMPatchErrorHandling(t *testing.T) { + // Test with invalid input that should trigger error logging + invalidInput := make(chan int) // channels can't be marshaled + + _, err := generateOLMPatch("error-test-version", invalidInput, "spec") + if err == nil { + t.Error("Expected error for invalid input, but got none") + } + + // The error should contain information about conversion failure + if !strings.Contains(err.Error(), "error converting to unstructured") { + t.Errorf("Expected error about conversion failure, got: %v", err) + } +} + +func TestApplyOperatorSpecLogging(t *testing.T) { + // This test demonstrates the logging functionality of ApplyOperatorSpec + // Note: This creates a mock scenario to test logging without requiring a real K8s cluster + + // Test that ApplyOperatorSpec handles nil configuration correctly with logging + client := &OperatorClient{} // Mock client for testing + + err := client.ApplyOperatorSpec(context.Background(), "test-field-manager", nil) + if err == nil { + t.Error("Expected error for nil applyConfiguration, but got none") + } + + // The error should contain the expected message + expectedError := "applyConfiguration must have a value" + if !strings.Contains(err.Error(), expectedError) { + t.Errorf("Expected error containing %q, got: %v", expectedError, err) + } +} + +func TestPatchOperatorStatusLogging(t *testing.T) { + // This test demonstrates the logging functionality of PatchOperatorStatus + // Note: This creates a mock scenario to test logging without requiring a real K8s cluster + + // Test that PatchOperatorStatus handles nil jsonPatch correctly with logging + client := &OperatorClient{} // Mock client for testing + + err := client.PatchOperatorStatus(context.Background(), nil) + if err == nil { + t.Error("Expected error for nil jsonPatch, but got none") + } + + // The error should contain the expected message + expectedError := "jsonPatch must have a value" + if !strings.Contains(err.Error(), expectedError) { + t.Errorf("Expected error containing %q, got: %v", expectedError, err) + } +} + +func TestConvertApplyConfigToOperatorSpec(t *testing.T) { + // Test the conversion from OperatorSpecApplyConfiguration to OperatorSpec + applyConfig := &operatorv1apply.OperatorSpecApplyConfiguration{} + applyConfig.WithManagementState(operatorv1.Managed) + applyConfig.WithLogLevel(operatorv1.Debug) + applyConfig.WithOperatorLogLevel(operatorv1.Normal) + + operatorSpec, err := convertApplyConfigToOperatorSpec(applyConfig) + if err != nil { + t.Fatalf("convertApplyConfigToOperatorSpec failed: %v", err) + } + + if operatorSpec.ManagementState != operatorv1.Managed { + t.Errorf("Expected ManagementState %s, got %s", operatorv1.Managed, operatorSpec.ManagementState) + } + + if operatorSpec.LogLevel != operatorv1.Debug { + t.Errorf("Expected LogLevel %s, got %s", operatorv1.Debug, operatorSpec.LogLevel) + } + + if operatorSpec.OperatorLogLevel != operatorv1.Normal { + t.Errorf("Expected OperatorLogLevel %s, got %s", operatorv1.Normal, operatorSpec.OperatorLogLevel) + } +} + +func TestOperatorSpecNeedsUpdate(t *testing.T) { + // Test the comparison function + baseSpec := operatorv1.OperatorSpec{ + ManagementState: operatorv1.Managed, + LogLevel: operatorv1.Debug, + OperatorLogLevel: operatorv1.Normal, + } + + // Test no changes needed + needsUpdate, err := operatorSpecNeedsUpdate(baseSpec, baseSpec) + if err != nil { + t.Fatalf("operatorSpecNeedsUpdate failed: %v", err) + } + if needsUpdate { + t.Error("Expected no update needed for identical specs") + } + + // Test management state change + changedSpec := baseSpec + changedSpec.ManagementState = operatorv1.Unmanaged + needsUpdate, err = operatorSpecNeedsUpdate(baseSpec, changedSpec) + if err != nil { + t.Fatalf("operatorSpecNeedsUpdate failed: %v", err) + } + if !needsUpdate { + t.Error("Expected update needed for different ManagementState") + } + + // Test log level change + changedSpec = baseSpec + changedSpec.LogLevel = operatorv1.Trace + needsUpdate, err = operatorSpecNeedsUpdate(baseSpec, changedSpec) + if err != nil { + t.Fatalf("operatorSpecNeedsUpdate failed: %v", err) + } + if !needsUpdate { + t.Error("Expected update needed for different LogLevel") + } +} + +func TestGenerateFinalizerPatch(t *testing.T) { + // Test generateFinalizerPatch function + testFinalizers := []string{"finalizer1", "finalizer2"} + + patchBytes, err := generateFinalizerPatch("test-version", testFinalizers) + if err != nil { + t.Fatalf("Error generating finalizer patch: %v", err) + } + + // Unmarshal the patch to verify its structure + var patchObj map[string]interface{} + if err := json.Unmarshal(patchBytes, &patchObj); err != nil { + t.Fatalf("Error unmarshaling finalizer patch: %v", err) + } + + // Verify top-level patch structure + if apiVersion, ok := patchObj["apiVersion"].(string); !ok || apiVersion != "operator.openshift.io/v1" { + t.Errorf("Expected apiVersion 'operator.openshift.io/v1', got %v", patchObj["apiVersion"]) + } + + if kind, ok := patchObj["kind"].(string); !ok || kind != "OLM" { + t.Errorf("Expected kind 'OLM', got %v", patchObj["kind"]) + } + + if resourceVersion, ok := patchObj["metadata"].(map[string]interface{})["resourceVersion"].(string); !ok || resourceVersion != "test-version" { + t.Errorf("Expected resourceVersion 'test-version', got %v", resourceVersion) + } + + // Verify finalizers field exists in metadata + metadata, ok := patchObj["metadata"].(map[string]interface{}) + if !ok { + t.Fatal("No metadata field in patch") + } + + finalizers, ok := metadata["finalizers"].([]interface{}) + if !ok { + t.Fatal("No finalizers field in metadata") + } + + if len(finalizers) != 2 { + t.Errorf("Expected 2 finalizers, got %d", len(finalizers)) + } + + // Verify finalizer content + if finalizers[0] != "finalizer1" || finalizers[1] != "finalizer2" { + t.Errorf("Expected finalizers [finalizer1, finalizer2], got %v", finalizers) + } + + // Verify that ONLY metadata.finalizers is set (no spec field should exist) + if _, hasSpec := patchObj["spec"]; hasSpec { + t.Error("Finalizer patch should not contain spec field") + } + + if _, hasStatus := patchObj["status"]; hasStatus { + t.Error("Finalizer patch should not contain status field") + } + + // Verify metadata only contains resourceVersion and finalizers + expectedMetadataFields := map[string]bool{"resourceVersion": false, "finalizers": false} + for field := range metadata { + if _, expected := expectedMetadataFields[field]; expected { + expectedMetadataFields[field] = true + } else { + t.Errorf("Unexpected metadata field %s in finalizer patch", field) + } + } + + // Check that all expected metadata fields were found + for field, found := range expectedMetadataFields { + if !found { + t.Errorf("Missing expected metadata field: %s", field) + } + } +} + +func TestGenerateFinalizerPatchWithEmptyFinalizers(t *testing.T) { + // Test with empty finalizers list + patchBytes, err := generateFinalizerPatch("empty-test", []string{}) + if err != nil { + t.Fatalf("Error generating empty finalizer patch: %v", err) + } + + var patchObj map[string]interface{} + if err := json.Unmarshal(patchBytes, &patchObj); err != nil { + t.Fatalf("Error unmarshaling empty finalizer patch: %v", err) + } + + metadata, ok := patchObj["metadata"].(map[string]interface{}) + if !ok { + t.Fatal("No metadata field in patch") + } + + finalizers, ok := metadata["finalizers"].([]interface{}) + if !ok { + t.Fatal("No finalizers field in metadata") + } + + if len(finalizers) != 0 { + t.Errorf("Expected 0 finalizers, got %d", len(finalizers)) + } +} + +func TestGenerateFinalizerPatchPreservesOtherFields(t *testing.T) { + // This test verifies that the finalizer patch only targets metadata.finalizers + // and would not overwrite other fields like spec when applied + + testFinalizers := []string{"test-finalizer"} + + patchBytes, err := generateFinalizerPatch("preserve-test", testFinalizers) + if err != nil { + t.Fatalf("Error generating finalizer patch: %v", err) + } + + var patchObj map[string]interface{} + if err := json.Unmarshal(patchBytes, &patchObj); err != nil { + t.Fatalf("Error unmarshaling finalizer patch: %v", err) + } + + // Verify that the patch only targets specific fields and won't overwrite others + expectedTopLevelFields := []string{"apiVersion", "kind", "metadata"} + for _, field := range expectedTopLevelFields { + if _, exists := patchObj[field]; !exists { + t.Errorf("Expected top-level field %s missing from finalizer patch", field) + } + } + + // Verify no unexpected top-level fields that would overwrite OLM fields + for field := range patchObj { + found := false + for _, expected := range expectedTopLevelFields { + if field == expected { + found = true + break + } + } + if !found { + t.Errorf("Unexpected top-level field %s in finalizer patch", field) + } + } + + // Specifically verify spec and status are NOT present + if _, hasSpec := patchObj["spec"]; hasSpec { + t.Error("Finalizer patch must not contain spec field to avoid overwriting OperatorSpec fields") + } + if _, hasStatus := patchObj["status"]; hasStatus { + t.Error("Finalizer patch must not contain status field to avoid overwriting OperatorStatus fields") + } +} + +// Helper function to get map keys for debugging +func getKeys(m map[string]interface{}) []string { + keys := make([]string, 0, len(m)) + for k := range m { + keys = append(keys, k) + } + return keys +} diff --git a/pkg/controller/builder.go b/pkg/controller/builder.go index 5d707271d..50be25579 100644 --- a/pkg/controller/builder.go +++ b/pkg/controller/builder.go @@ -22,8 +22,6 @@ import ( "golang.org/x/text/cases" "golang.org/x/text/language" - appsv1 "k8s.io/api/apps/v1" - corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -126,11 +124,13 @@ func (b *Builder) BuildControllers(subDirectories ...string) (map[string]factory b.Clients.KubeInformerFactory.Apps().V1().Deployments(), []factory.Informer{ b.Clients.ProxyClient.Informer(), + b.Clients.OperatorClient.Informer(), }, []deploymentcontroller.ManifestHookFunc{ replaceVerbosityHook("${LOG_VERBOSITY}"), }, UpdateDeploymentProxyHook(b.Clients.ProxyClient), + UpdateDeploymentObservedConfigHook(b.Clients.OperatorClient), ) return nil } @@ -259,61 +259,3 @@ func replaceVerbosityHook(placeholder string) deploymentcontroller.ManifestHookF return []byte(newDeployment), nil } } - -func updateEnv(con *corev1.Container, env corev1.EnvVar) error { - for _, e := range con.Env { - if e.Name == env.Name { - return fmt.Errorf("unexpected environment variable %q=%q in container %q while building manifests", e.Name, e.Value, con.Name) - } - } - if env.Value == "" { - return nil - } - klog.FromContext(context.Background()).WithName("builder").V(4).Info("Updated environment", "container", con.Name, "key", env.Name, "value", env.Value) - con.Env = append(con.Env, env) - return nil -} - -func setContainerEnv(con *corev1.Container, envs []corev1.EnvVar) error { - for _, env := range envs { - if err := updateEnv(con, env); err != nil { - return err - } - } - return nil -} - -func UpdateDeploymentProxyHook(pc clients.ProxyClientInterface) deploymentcontroller.DeploymentHookFunc { - return func(_ *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error { - klog.FromContext(context.Background()).WithName("builder").V(1).Info("ProxyHook updating environment", "deployment", deployment.Name) - proxyConfig, err := pc.Get("cluster") - if err != nil { - return fmt.Errorf("error getting proxies.config.openshift.io/cluster: %w", err) - } - - vars := []corev1.EnvVar{ - {Name: HTTPSProxy, Value: proxyConfig.Status.HTTPSProxy}, - {Name: HTTPProxy, Value: proxyConfig.Status.HTTPProxy}, - {Name: NoProxy, Value: proxyConfig.Status.NoProxy}, - } - - var errs []error - for i := range deployment.Spec.Template.Spec.InitContainers { - err = setContainerEnv(&deployment.Spec.Template.Spec.InitContainers[i], vars) - if err != nil { - errs = append(errs, err) - } - } - for i := range deployment.Spec.Template.Spec.Containers { - err = setContainerEnv(&deployment.Spec.Template.Spec.Containers[i], vars) - if err != nil { - errs = append(errs, err) - } - } - if len(errs) > 0 { - return errors.Join(errs...) - } - - return nil - } -} diff --git a/pkg/controller/builder_test.go b/pkg/controller/builder_test.go index 63e90cd9f..6ecc4fd4b 100644 --- a/pkg/controller/builder_test.go +++ b/pkg/controller/builder_test.go @@ -3,7 +3,6 @@ package controller import ( "testing" - configv1 "github.com/openshift/api/config/v1" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -72,69 +71,3 @@ func TestControllerNameForObject(t *testing.T) { }) } } - -type MockProxyClient struct { - configv1.Proxy -} - -func (m *MockProxyClient) Get(_ string) (*configv1.Proxy, error) { - return &m.Proxy, nil -} - -func TestProxyUpdateEnv(t *testing.T) { - mpc := MockProxyClient{ - Proxy: configv1.Proxy{ - Status: configv1.ProxyStatus{ - HTTPProxy: HTTPProxy, - HTTPSProxy: HTTPSProxy, - NoProxy: NoProxy, - }, - }, - } - - dep := appsv1.Deployment{ - Spec: appsv1.DeploymentSpec{ - Template: corev1.PodTemplateSpec{ - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - { - Name: "test", - }, - }, - }, - }, - }, - } - - update := UpdateDeploymentProxyHook(&mpc) - err := update(nil, &dep) - if err != nil { - t.Fatalf("unexpected error in first update: %v", err) - } - if len(dep.Spec.Template.Spec.Containers[0].Env) != 3 { - t.Fatalf("environment length not 3: %+v", dep) - } - - // We want to make sure the order is preserved, so check explicitly - expectedVars := []corev1.EnvVar{ - {Name: HTTPSProxy, Value: HTTPSProxy}, - {Name: HTTPProxy, Value: HTTPProxy}, - {Name: NoProxy, Value: NoProxy}, - } - validateEnvVarsOrFail(t, expectedVars, dep.Spec.Template.Spec.Containers[0].Env) - - err = update(nil, &dep) - if err == nil { - t.Fatal("no error in second update") - } - // Make sure the Deployment is unchanged - validateEnvVarsOrFail(t, expectedVars, dep.Spec.Template.Spec.Containers[0].Env) -} - -func validateEnvVarsOrFail(t *testing.T, in, expected []corev1.EnvVar) { - for i := range in { - if in[i] != expected[i] { - t.Fatalf("iter %d: expected: %+v, got: %+v", i, in[i], expected[i]) - } - } -} diff --git a/pkg/controller/observedconfig_hook.go b/pkg/controller/observedconfig_hook.go new file mode 100644 index 000000000..84ebbf520 --- /dev/null +++ b/pkg/controller/observedconfig_hook.go @@ -0,0 +1,141 @@ +// Package controller provides deployment hooks for configuring TLS security profiles +// and proxy settings from OpenShift cluster configuration. +package controller + +import ( + "encoding/json" + "errors" + "fmt" + "strings" + + operatorv1 "github.com/openshift/api/operator/v1" + "github.com/openshift/cluster-olm-operator/pkg/clients" + "github.com/openshift/library-go/pkg/operator/deploymentcontroller" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/klog/v2" +) + +// UpdateDeploymentObservedConfigHook creates a deployment hook that reads observedConfig +// from the olms.operator.openshift.io resource and extracts TLS configuration +func UpdateDeploymentObservedConfigHook(_ *clients.OperatorClient) deploymentcontroller.DeploymentHookFunc { + return func(operatorSpec *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error { + klog.V(1).Infof("ObservedConfigHook updating arguments for deployment %s", deployment.Name) + + // Extract TLS configuration from observedConfig + cfg, err := extractTLSConfigFromObservedConfig(operatorSpec) + if err != nil { + klog.V(2).Infof("Failed to extract TLS config from observedConfig: %v", err) + // Don't return error - just log and continue without TLS env vars + return nil + } + + if len(cfg) == 0 { + klog.V(2).Info("No TLS configuration found in observedConfig") + return nil + } + + klog.V(2).Infof("Found %d TLS environment variables from observedConfig", len(cfg)) + + var errs []error + for i := range deployment.Spec.Template.Spec.Containers { + err = setContainerArgs(&deployment.Spec.Template.Spec.Containers[i], cfg) + if err != nil { + errs = append(errs, err) + } + } + if len(errs) > 0 { + return errors.Join(errs...) + } + + return nil + } +} + +// extractTLSConfigFromObservedConfig extracts TLS security profile configuration from +// the operator's observedConfig and returns environment variables +func extractTLSConfigFromObservedConfig(operatorSpec *operatorv1.OperatorSpec) ([]string, error) { + if operatorSpec == nil || len(operatorSpec.ObservedConfig.Raw) == 0 { + return nil, nil + } + + // Parse the observedConfig JSON + var observedConfig map[string]interface{} + if err := json.Unmarshal(operatorSpec.ObservedConfig.Raw, &observedConfig); err != nil { + return nil, fmt.Errorf("error unmarshaling observedConfig: %w", err) + } + + // Extract TLS security profile configuration + tlsProfile, found, err := unstructured.NestedMap(observedConfig, "olmTLSSecurityProfile") + if err != nil { + return nil, fmt.Errorf("error accessing olmTLSSecurityProfile: %w", err) + } + if !found { + klog.V(3).Info("No olmTLSSecurityProfile found in observedConfig") + return nil, nil + } + + var args []string + + // Extract minTLSVersion + if minTLSVersion, found, err := unstructured.NestedString(tlsProfile, "minTLSVersion"); err != nil { + return nil, fmt.Errorf("error accessing minTLSVersion: %w", err) + } else if found && minTLSVersion != "" { + translateTLSVersions := map[string]string{ + "VersionTLS1": "TLSv1.0", + "VersionTLS10": "TLSv1.0", + "VersionTLS11": "TLSv1.1", + "VersionTLS12": "TLSv1.2", + "VersionTLS13": "TLSv1.3", + } + argTLSVersion, ok := translateTLSVersions[minTLSVersion] + if !ok { + return nil, fmt.Errorf("unknown TLS version: %q", minTLSVersion) + } + args = append(args, fmt.Sprintf("--tls-custom-version=%s", argTLSVersion)) + klog.V(3).Infof("Extracted minTLSVersion: %s", argTLSVersion) + } + + // Extract cipherSuites + if cipherSuites, found, err := unstructured.NestedStringSlice(tlsProfile, "cipherSuites"); err != nil { + return nil, fmt.Errorf("error accessing cipherSuites: %w", err) + } else if found && len(cipherSuites) > 0 { + // Join cipher suites with commas for environment variable + cipherSuitesStr := strings.Join(cipherSuites, ",") + args = append(args, fmt.Sprintf("--tls-custom-ciphers=%s", cipherSuitesStr)) + klog.V(3).Infof("Extracted %d cipher suites: %s", len(cipherSuites), cipherSuitesStr) + } + + switch len(args) { + case 0: + case 2: + args = append(args, "--tls-profile=custom") + default: + return nil, fmt.Errorf("invalid observedConfig for TLS, missing argument") + } + + return args, nil +} + +func updateArgs(con *corev1.Container, arg string) error { + splitStrings := strings.Split(arg, "=") + prefix := splitStrings[0] + for _, e := range con.Args { + if strings.HasPrefix(e, prefix) { + return fmt.Errorf("unexpected argument %q in container %q while building manifests", arg, con.Name) + } + } + klog.V(4).Infof("Updated arguments for container %s: %s", con.Name, arg) + con.Args = append(con.Args, arg) + return nil +} + +func setContainerArgs(con *corev1.Container, args []string) error { + for _, arg := range args { + if err := updateArgs(con, arg); err != nil { + return err + } + } + return nil +} diff --git a/pkg/controller/observedconfig_hook_test.go b/pkg/controller/observedconfig_hook_test.go new file mode 100644 index 000000000..7c24c5754 --- /dev/null +++ b/pkg/controller/observedconfig_hook_test.go @@ -0,0 +1,205 @@ +package controller + +import ( + "testing" + + operatorv1 "github.com/openshift/api/operator/v1" + "k8s.io/apimachinery/pkg/runtime" +) + +func TestUpdateDeploymentObservedConfigHook(t *testing.T) { + tests := []struct { + name string + operatorSpec *operatorv1.OperatorSpec + expectedArgs []string + expectError bool + }{ + { + name: "valid TLS configuration", + operatorSpec: &operatorv1.OperatorSpec{ + ObservedConfig: runtime.RawExtension{ + Raw: []byte(`{ + "olmTLSSecurityProfile": { + "minTLSVersion": "VersionTLS12", + "cipherSuites": ["TLS_AES_128_GCM_SHA256", "TLS_AES_256_GCM_SHA384"] + } + }`), + }, + }, + expectedArgs: []string{ + "--tls-custom-version=TLSv1.2", + "--tls-custom-ciphers=TLS_AES_128_GCM_SHA256,TLS_AES_256_GCM_SHA384", + "--tls-profile=custom", + }, + expectError: false, + }, + { + name: "empty observedConfig", + operatorSpec: &operatorv1.OperatorSpec{ + ObservedConfig: runtime.RawExtension{}, + }, + expectedArgs: []string{}, + expectError: false, + }, + { + name: "no TLS profile in observedConfig", + operatorSpec: &operatorv1.OperatorSpec{ + ObservedConfig: runtime.RawExtension{ + Raw: []byte(`{"someOtherConfig": "value"}`), + }, + }, + expectedArgs: []string{}, + expectError: false, + }, + { + name: "only minTLSVersion", + operatorSpec: &operatorv1.OperatorSpec{ + ObservedConfig: runtime.RawExtension{ + Raw: []byte(`{ + "olmTLSSecurityProfile": { + "minTLSVersion": "VersionTLS13" + } + }`), + }, + }, + expectedArgs: nil, + expectError: true, + }, + { + name: "only cipherSuites", + operatorSpec: &operatorv1.OperatorSpec{ + ObservedConfig: runtime.RawExtension{ + Raw: []byte(`{ + "olmTLSSecurityProfile": { + "cipherSuites": ["TLS_AES_128_GCM_SHA256"] + } + }`), + }, + }, + expectedArgs: nil, + expectError: true, + }, + { + name: "TLS version translation", + operatorSpec: &operatorv1.OperatorSpec{ + ObservedConfig: runtime.RawExtension{ + Raw: []byte(`{ + "olmTLSSecurityProfile": { + "minTLSVersion": "VersionTLS11" + } + }`), + }, + }, + expectedArgs: nil, + expectError: true, + }, + { + name: "valid complete TLS configuration with custom profile", + operatorSpec: &operatorv1.OperatorSpec{ + ObservedConfig: runtime.RawExtension{ + Raw: []byte(`{ + "olmTLSSecurityProfile": { + "minTLSVersion": "VersionTLS11", + "cipherSuites": ["TLS_AES_128_GCM_SHA256", "TLS_AES_256_GCM_SHA384"] + } + }`), + }, + }, + expectedArgs: []string{ + "--tls-custom-version=TLSv1.1", + "--tls-custom-ciphers=TLS_AES_128_GCM_SHA256,TLS_AES_256_GCM_SHA384", + "--tls-profile=custom", + }, + expectError: false, + }, + { + name: "nil operatorSpec", + operatorSpec: nil, + expectedArgs: []string{}, + expectError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Call the hook function directly using the extractTLSConfigFromObservedConfig function + args, err := extractTLSConfigFromObservedConfig(tt.operatorSpec) + if tt.expectError && err == nil { + t.Fatal("expected error but got none") + } + if !tt.expectError && err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Verify the arguments + if len(args) != len(tt.expectedArgs) { + t.Fatalf("expected %d args, got %d", len(tt.expectedArgs), len(args)) + } + + for i, expected := range tt.expectedArgs { + if args[i] != expected { + t.Errorf("arg %d: expected %q, got %q", i, expected, args[i]) + } + } + }) + } +} + +func TestExtractTLSConfigFromObservedConfig(t *testing.T) { + tests := []struct { + name string + observedConfig string + expectedArgs []string + expectError bool + }{ + { + name: "invalid JSON", + observedConfig: `{ + "olmTLSSecurityProfile": { + "minTLSVersion": "VersionTLS12", + "cipherSuites": [ + } + }`, + expectedArgs: nil, + expectError: true, + }, + { + name: "empty TLS profile", + observedConfig: `{ + "olmTLSSecurityProfile": {} + }`, + expectedArgs: []string{}, + expectError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + operatorSpec := &operatorv1.OperatorSpec{ + ObservedConfig: runtime.RawExtension{ + Raw: []byte(tt.observedConfig), + }, + } + + args, err := extractTLSConfigFromObservedConfig(operatorSpec) + if tt.expectError && err == nil { + t.Fatal("expected error but got none") + } + if !tt.expectError && err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if !tt.expectError { + if len(args) != len(tt.expectedArgs) { + t.Fatalf("expected %d args, got %d", len(tt.expectedArgs), len(args)) + } + + for i, expected := range tt.expectedArgs { + if args[i] != expected { + t.Errorf("arg %d: expected %q, got %q", i, expected, args[i]) + } + } + } + }) + } +} diff --git a/pkg/controller/proxy_hook.go b/pkg/controller/proxy_hook.go new file mode 100644 index 000000000..4135e14aa --- /dev/null +++ b/pkg/controller/proxy_hook.go @@ -0,0 +1,81 @@ +// Package controller provides deployment hooks for configuring TLS security profiles +// and proxy settings from OpenShift cluster configuration. +package controller + +import ( + "context" + "errors" + "fmt" + + operatorv1 "github.com/openshift/api/operator/v1" + "github.com/openshift/cluster-olm-operator/pkg/clients" + "github.com/openshift/library-go/pkg/operator/deploymentcontroller" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + "k8s.io/klog/v2" +) + +// Proxy environment variable constants +const ( + HTTPProxy = "HTTP_PROXY" + HTTPSProxy = "HTTPS_PROXY" + NoProxy = "NO_PROXY" +) + +func UpdateDeploymentProxyHook(pc clients.ProxyClientInterface) deploymentcontroller.DeploymentHookFunc { + return func(_ *operatorv1.OperatorSpec, deployment *appsv1.Deployment) error { + klog.FromContext(context.Background()).WithName("builder").V(1).Info("ProxyHook updating environment", "deployment", deployment.Name) + proxyConfig, err := pc.Get("cluster") + if err != nil { + return fmt.Errorf("error getting proxies.config.openshift.io/cluster: %w", err) + } + + vars := []corev1.EnvVar{ + {Name: HTTPSProxy, Value: proxyConfig.Status.HTTPSProxy}, + {Name: HTTPProxy, Value: proxyConfig.Status.HTTPProxy}, + {Name: NoProxy, Value: proxyConfig.Status.NoProxy}, + } + + var errs []error + for i := range deployment.Spec.Template.Spec.InitContainers { + err = setContainerEnv(&deployment.Spec.Template.Spec.InitContainers[i], vars) + if err != nil { + errs = append(errs, err) + } + } + for i := range deployment.Spec.Template.Spec.Containers { + err = setContainerEnv(&deployment.Spec.Template.Spec.Containers[i], vars) + if err != nil { + errs = append(errs, err) + } + } + if len(errs) > 0 { + return errors.Join(errs...) + } + + return nil + } +} + +func updateEnv(con *corev1.Container, env corev1.EnvVar) error { + for _, e := range con.Env { + if e.Name == env.Name { + return fmt.Errorf("unexpected environment variable %q=%q in container %q while building manifests", e.Name, e.Value, con.Name) + } + } + if env.Value == "" { + return nil + } + klog.FromContext(context.Background()).WithName("builder").V(4).Info("Updated environment", "container", con.Name, "key", env.Name, "value", env.Value) + con.Env = append(con.Env, env) + return nil +} + +func setContainerEnv(con *corev1.Container, envs []corev1.EnvVar) error { + for _, env := range envs { + if err := updateEnv(con, env); err != nil { + return err + } + } + return nil +} diff --git a/pkg/controller/proxy_hook_test.go b/pkg/controller/proxy_hook_test.go new file mode 100644 index 000000000..f92d9234d --- /dev/null +++ b/pkg/controller/proxy_hook_test.go @@ -0,0 +1,75 @@ +package controller + +import ( + "testing" + + configv1 "github.com/openshift/api/config/v1" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" +) + +type MockProxyClient struct { + configv1.Proxy +} + +func (m *MockProxyClient) Get(_ string) (*configv1.Proxy, error) { + return &m.Proxy, nil +} + +func TestProxyUpdateEnv(t *testing.T) { + mpc := MockProxyClient{ + Proxy: configv1.Proxy{ + Status: configv1.ProxyStatus{ + HTTPProxy: HTTPProxy, + HTTPSProxy: HTTPSProxy, + NoProxy: NoProxy, + }, + }, + } + + dep := appsv1.Deployment{ + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "test", + }, + }, + }, + }, + }, + } + + update := UpdateDeploymentProxyHook(&mpc) + err := update(nil, &dep) + if err != nil { + t.Fatalf("unexpected error in first update: %v", err) + } + if len(dep.Spec.Template.Spec.Containers[0].Env) != 3 { + t.Fatalf("environment length not 3: %+v", dep) + } + + // We want to make sure the order is preserved, so check explicitly + expectedVars := []corev1.EnvVar{ + {Name: HTTPSProxy, Value: HTTPSProxy}, + {Name: HTTPProxy, Value: HTTPProxy}, + {Name: NoProxy, Value: NoProxy}, + } + validateEnvVarsOrFail(t, expectedVars, dep.Spec.Template.Spec.Containers[0].Env) + + err = update(nil, &dep) + if err == nil { + t.Fatal("no error in second update") + } + // Make sure the Deployment is unchanged + validateEnvVarsOrFail(t, expectedVars, dep.Spec.Template.Spec.Containers[0].Env) +} + +func validateEnvVarsOrFail(t *testing.T, in, expected []corev1.EnvVar) { + for i := range in { + if in[i] != expected[i] { + t.Fatalf("iter %d: expected: %+v, got: %+v", i, in[i], expected[i]) + } + } +} diff --git a/pkg/controller/proxycontroller.go b/pkg/controller/proxycontroller.go index c6d4a6f39..d84a94524 100644 --- a/pkg/controller/proxycontroller.go +++ b/pkg/controller/proxycontroller.go @@ -14,12 +14,6 @@ import ( "github.com/openshift/cluster-olm-operator/pkg/clients" ) -const ( - HTTPProxy = "HTTP_PROXY" - HTTPSProxy = "HTTPS_PROXY" - NoProxy = "NO_PROXY" -) - func NewProxyController(name string, proxyClient *clients.ProxyClient, operatorClient *clients.OperatorClient, eventRecorder events.Recorder) factory.Controller { c := proxyController{ name: name, diff --git a/pkg/controller/tlsobserver.go b/pkg/controller/tlsobserver.go new file mode 100644 index 000000000..663aac3e1 --- /dev/null +++ b/pkg/controller/tlsobserver.go @@ -0,0 +1,166 @@ +package controller + +import ( + configinformers "github.com/openshift/client-go/config/informers/externalversions" + configlistersv1 "github.com/openshift/client-go/config/listers/config/v1" + "github.com/openshift/library-go/pkg/controller/factory" + "github.com/openshift/library-go/pkg/operator/configobserver" + "github.com/openshift/library-go/pkg/operator/configobserver/apiserver" + "github.com/openshift/library-go/pkg/operator/events" + "github.com/openshift/library-go/pkg/operator/resourcesynccontroller" + "github.com/openshift/library-go/pkg/operator/v1helpers" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/client-go/tools/cache" + "k8s.io/klog/v2" +) + +// TLSSecurityProfileConfigPath returns the path for the observed TLS security profile configuration. +func TLSSecurityProfileConfigPath() []string { + return []string{"olmTLSSecurityProfile"} +} + +// TLSMinVersionPath returns the path for the observed minimum TLS version. +func TLSMinVersionPath() []string { + return []string{"olmTLSSecurityProfile", "minTLSVersion"} +} + +// TLSCipherSuitesPath returns the path for the observed TLS cipher suites. +func TLSCipherSuitesPath() []string { + return []string{"olmTLSSecurityProfile", "cipherSuites"} +} + +// OLMConfigObserverListers implements the configobserver.Listers interface and +// apiserver.APIServerLister for use with the library-go ObserveTLSSecurityProfile +type OLMConfigObserverListers struct { + APIServerListerImpl configlistersv1.APIServerLister + ResourceSync resourcesynccontroller.ResourceSyncer + PreRunCachesSynced []cache.InformerSynced +} + +func (l OLMConfigObserverListers) APIServerLister() configlistersv1.APIServerLister { + klog.Info("TLS observer: APIServerLister() called") + return l.APIServerListerImpl +} + +func (l OLMConfigObserverListers) ResourceSyncer() resourcesynccontroller.ResourceSyncer { + klog.Info("TLS observer: ResourceSyncer() called") + return l.ResourceSync +} + +func (l OLMConfigObserverListers) PreRunHasSynced() []cache.InformerSynced { + klog.Infof("TLS observer: PreRunHasSynced() called, returning %d synced informers", len(l.PreRunCachesSynced)) + return l.PreRunCachesSynced +} + +// TLSObserverController creates a config observer controller that observes TLS security profiles +// using the library-go ObserveTLSSecurityProfile function +type TLSObserverController struct { + factory.Controller +} + +// NewTLSObserverController returns a new TLS observer controller using library-go patterns +func NewTLSObserverController( + name string, + operatorClient v1helpers.OperatorClient, + configInformers configinformers.SharedInformerFactory, + eventRecorder events.Recorder, +) *TLSObserverController { + klog.Infof("Creating TLS observer controller: %s", name) + + informers := []factory.Informer{ + operatorClient.Informer(), + configInformers.Config().V1().APIServers().Informer(), + } + + klog.Infof("TLS observer controller %s: registering informers for operator client and APIServer config", name) + + c := &TLSObserverController{ + Controller: configobserver.NewConfigObserver( + name, + operatorClient, + eventRecorder.WithComponentSuffix("tls-config-observer-controller"), + OLMConfigObserverListers{ + APIServerListerImpl: configInformers.Config().V1().APIServers().Lister(), + PreRunCachesSynced: []cache.InformerSynced{ + operatorClient.Informer().HasSynced, + configInformers.Config().V1().APIServers().Informer().HasSynced, + }, + }, + informers, + observeTLSSecurityProfileForOLM, + ), + } + + klog.Infof("TLS observer controller %s: successfully created with library-go config observer", name) + return c +} + +// observeTLSSecurityProfileForOLM uses the library-go ObserveTLSSecurityProfile function +// with OLM-specific configuration paths +func observeTLSSecurityProfileForOLM(genericListers configobserver.Listers, recorder events.Recorder, existingConfig map[string]interface{}) (map[string]interface{}, []error) { + klog.Info("TLS observer: starting TLS security profile observation") + + // Log the configuration paths being used + minTLSPath := TLSMinVersionPath() + cipherSuitesPath := TLSCipherSuitesPath() + klog.Infof("TLS observer: using minTLSVersion path: %v", minTLSPath) + klog.Infof("TLS observer: using cipherSuites path: %v", cipherSuitesPath) + + // Log current existing configuration if present + if len(existingConfig) > 0 { + klog.Infof("TLS observer: existing config keys: %v", getMapKeys(existingConfig)) + if currentMinTLS, found, _ := unstructured.NestedString(existingConfig, minTLSPath...); found { + klog.Infof("TLS observer: current minTLSVersion: %s", currentMinTLS) + } + if currentCiphers, found, _ := unstructured.NestedStringSlice(existingConfig, cipherSuitesPath...); found { + klog.Infof("TLS observer: current cipherSuites count: %d", len(currentCiphers)) + } + } else { + klog.Info("TLS observer: no existing configuration found") + } + + // Call the library-go function + observedConfig, errs := apiserver.ObserveTLSSecurityProfileWithPaths( + genericListers, + recorder, + existingConfig, + minTLSPath, + cipherSuitesPath, + ) + + // Log the results + if len(errs) > 0 { + klog.Warningf("TLS observer: encountered %d errors during observation", len(errs)) + for i, err := range errs { + klog.Warningf("TLS observer: error %d: %v", i+1, err) + } + } else { + klog.Info("TLS observer: observation completed without errors") + } + + // Log observed configuration + if len(observedConfig) > 0 { + klog.Infof("TLS observer: observed config keys: %v", getMapKeys(observedConfig)) + if newMinTLS, found, _ := unstructured.NestedString(observedConfig, minTLSPath...); found { + klog.Infof("TLS observer: observed minTLSVersion: %s", newMinTLS) + } + if newCiphers, found, _ := unstructured.NestedStringSlice(observedConfig, cipherSuitesPath...); found { + klog.Infof("TLS observer: observed %d cipher suites", len(newCiphers)) + klog.Infof("TLS observer: cipher suites: %v", newCiphers) + } + } else { + klog.Info("TLS observer: no configuration observed") + } + + klog.Info("TLS observer: completed TLS security profile observation") + return observedConfig, errs +} + +// getMapKeys is a helper function to extract keys from a map for logging purposes +func getMapKeys(m map[string]interface{}) []string { + keys := make([]string, 0, len(m)) + for k := range m { + keys = append(keys, k) + } + return keys +} diff --git a/pkg/controller/tlsobserver_test.go b/pkg/controller/tlsobserver_test.go new file mode 100644 index 000000000..4d87fc3e7 --- /dev/null +++ b/pkg/controller/tlsobserver_test.go @@ -0,0 +1,110 @@ +package controller + +import ( + "testing" + + configv1 "github.com/openshift/api/config/v1" +) + +func TestTLSObserverPaths(t *testing.T) { + // Test that the configuration paths are correctly defined + expectedMinTLSPath := []string{"olmTLSSecurityProfile", "minTLSVersion"} + expectedCipherSuitesPath := []string{"olmTLSSecurityProfile", "cipherSuites"} + + if !equalStringSlices(TLSMinVersionPath(), expectedMinTLSPath) { + t.Errorf("TLSMinVersionPath() = %v, want %v", TLSMinVersionPath(), expectedMinTLSPath) + } + + if !equalStringSlices(TLSCipherSuitesPath(), expectedCipherSuitesPath) { + t.Errorf("TLSCipherSuitesPath() = %v, want %v", TLSCipherSuitesPath(), expectedCipherSuitesPath) + } +} + +func TestTLSSecurityProfileTypes(t *testing.T) { + // Test that TLS security profile types are available + types := []configv1.TLSProfileType{ + configv1.TLSProfileOldType, + configv1.TLSProfileIntermediateType, + configv1.TLSProfileModernType, + configv1.TLSProfileCustomType, + } + + expectedTypes := []string{"Old", "Intermediate", "Modern", "Custom"} + + for i, profileType := range types { + if string(profileType) != expectedTypes[i] { + t.Errorf("TLS profile type %d = %s, want %s", i, string(profileType), expectedTypes[i]) + } + } + + // Test that TLS profiles map exists + if configv1.TLSProfiles[configv1.TLSProfileIntermediateType] == nil { + t.Errorf("TLSProfiles map missing Intermediate profile") + } +} + +func TestLibraryGoIntegration(t *testing.T) { + // Test that we can access the library-go apiserver package + // This verifies that the vendor update was successful + if configv1.TLSProfiles[configv1.TLSProfileIntermediateType] == nil { + t.Errorf("TLSProfiles map missing Intermediate profile") + } + + // Test that we have the expected TLS profile structure + intermediateProfile := configv1.TLSProfiles[configv1.TLSProfileIntermediateType] + if intermediateProfile.MinTLSVersion != configv1.VersionTLS12 { + t.Errorf("Intermediate profile minTLSVersion = %s, want VersionTLS12", intermediateProfile.MinTLSVersion) + } + if len(intermediateProfile.Ciphers) == 0 { + t.Errorf("Intermediate profile has no cipher suites") + } +} + +func TestGetMapKeys(t *testing.T) { + // Test the helper function used for logging + testMap := map[string]interface{}{ + "key1": "value1", + "key2": 42, + "key3": []string{"a", "b"}, + } + + keys := getMapKeys(testMap) + if len(keys) != 3 { + t.Errorf("getMapKeys() returned %d keys, want 3", len(keys)) + } + + // Check that all expected keys are present (order may vary) + expectedKeys := map[string]bool{"key1": false, "key2": false, "key3": false} + for _, key := range keys { + if _, exists := expectedKeys[key]; exists { + expectedKeys[key] = true + } else { + t.Errorf("getMapKeys() returned unexpected key: %s", key) + } + } + + // Check that all expected keys were found + for key, found := range expectedKeys { + if !found { + t.Errorf("getMapKeys() missing expected key: %s", key) + } + } + + // Test empty map + emptyKeys := getMapKeys(map[string]interface{}{}) + if len(emptyKeys) != 0 { + t.Errorf("getMapKeys() for empty map returned %d keys, want 0", len(emptyKeys)) + } +} + +func equalStringSlices(a, b []string) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} diff --git a/vendor/github.com/openshift/library-go/pkg/operator/configobserver/apiserver/OWNERS b/vendor/github.com/openshift/library-go/pkg/operator/configobserver/apiserver/OWNERS new file mode 100644 index 000000000..582d67101 --- /dev/null +++ b/vendor/github.com/openshift/library-go/pkg/operator/configobserver/apiserver/OWNERS @@ -0,0 +1,8 @@ +reviewers: + - tkashem + - p0lyn0mial + - sttts +approvers: + - tkashem + - p0lyn0mial + - sttts diff --git a/vendor/github.com/openshift/library-go/pkg/operator/configobserver/apiserver/listers.go b/vendor/github.com/openshift/library-go/pkg/operator/configobserver/apiserver/listers.go new file mode 100644 index 000000000..2c0d1bda3 --- /dev/null +++ b/vendor/github.com/openshift/library-go/pkg/operator/configobserver/apiserver/listers.go @@ -0,0 +1,9 @@ +package apiserver + +import ( + configlistersv1 "github.com/openshift/client-go/config/listers/config/v1" +) + +type APIServerLister interface { + APIServerLister() configlistersv1.APIServerLister +} diff --git a/vendor/github.com/openshift/library-go/pkg/operator/configobserver/apiserver/observe_audit.go b/vendor/github.com/openshift/library-go/pkg/operator/configobserver/apiserver/observe_audit.go new file mode 100644 index 000000000..39aa79cb6 --- /dev/null +++ b/vendor/github.com/openshift/library-go/pkg/operator/configobserver/apiserver/observe_audit.go @@ -0,0 +1,88 @@ +package apiserver + +import ( + "fmt" + + k8serrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/klog/v2" + + "github.com/openshift/library-go/pkg/operator/configobserver" + "github.com/openshift/library-go/pkg/operator/events" +) + +// AuditPolicyPathGetterFunc allows the observer to be agnostic of the source of audit profile(s). +// The function returns the path to the audit policy file (associated with the +// given profile) in the static manifest folder. +type AuditPolicyPathGetterFunc func(profile string) (string, error) + +// NewAuditObserver returns an ObserveConfigFunc that observes the audit field of the APIServer resource +// and sets the apiServerArguments:audit-policy-file field for the apiserver appropriately. +func NewAuditObserver(pathGetter AuditPolicyPathGetterFunc) configobserver.ObserveConfigFunc { + var ( + apiServerArgumentsAuditPath = []string{"apiServerArguments", "audit-policy-file"} + ) + + return func(genericListers configobserver.Listers, recorder events.Recorder, existingConfig map[string]interface{}) (observed map[string]interface{}, _ []error) { + defer func() { + observed = configobserver.Pruned(observed, apiServerArgumentsAuditPath) + }() + + errs := []error{} + + // if the function encounters an error it returns existing/current config, which means that + // some other entity (default config in bindata ) must ensure to default the configuration. + // otherwise, the apiserver won't have a path to audit policy file and it will fail to start. + listers := genericListers.(APIServerLister) + apiServer, err := listers.APIServerLister().Get("cluster") + if err != nil { + if k8serrors.IsNotFound(err) { + klog.Warningf("apiserver.config.openshift.io/cluster: not found") + + return existingConfig, errs + } + + return existingConfig, append(errs, err) + } + + desiredProfile := string(apiServer.Spec.Audit.Profile) + if len(desiredProfile) == 0 { + // The specified Profile is empty, so let the defaulting layer choose a default for us. + return map[string]interface{}{}, errs + } + + desiredAuditPolicyPath, err := pathGetter(desiredProfile) + if err != nil { + return existingConfig, append(errs, fmt.Errorf("audit profile is not valid name=%s", desiredProfile)) + } + + currentAuditPolicyPath, err := getCurrentPolicyPath(existingConfig, apiServerArgumentsAuditPath...) + if err != nil { + return existingConfig, append(errs, fmt.Errorf("audit profile is not valid name=%s", desiredProfile)) + } + if desiredAuditPolicyPath == currentAuditPolicyPath { + return existingConfig, errs + } + + // we have a change of audit policy here! + observedConfig := map[string]interface{}{} + if err := unstructured.SetNestedStringSlice(observedConfig, []string{desiredAuditPolicyPath}, apiServerArgumentsAuditPath...); err != nil { + return existingConfig, append(errs, fmt.Errorf("failed to set desired audit profile in observed config name=%s", desiredProfile)) + } + + recorder.Eventf("ObserveAPIServerArgumentsAudit", "audit policy has been set to profile=%s", desiredProfile) + return observedConfig, errs + } +} + +func getCurrentPolicyPath(existing map[string]interface{}, fields ...string) (string, error) { + current, _, err := unstructured.NestedStringSlice(existing, fields...) + if err != nil { + return "", err + } + if len(current) == 0 { + return "", nil + } + + return current[0], nil +} diff --git a/vendor/github.com/openshift/library-go/pkg/operator/configobserver/apiserver/observe_cors.go b/vendor/github.com/openshift/library-go/pkg/operator/configobserver/apiserver/observe_cors.go new file mode 100644 index 000000000..f8868aa2a --- /dev/null +++ b/vendor/github.com/openshift/library-go/pkg/operator/configobserver/apiserver/observe_cors.go @@ -0,0 +1,75 @@ +package apiserver + +import ( + "k8s.io/klog/v2" + + "github.com/openshift/library-go/pkg/operator/configobserver" + "github.com/openshift/library-go/pkg/operator/events" + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/util/sets" +) + +var clusterDefaultCORSAllowedOrigins = []string{ + `//127\.0\.0\.1(:|$)`, + `//localhost(:|$)`, +} + +// ObserveAdditionalCORSAllowedOrigins observes the additionalCORSAllowedOrigins field +// of the APIServer resource and sets the corsAllowedOrigins field of observedConfig +func ObserveAdditionalCORSAllowedOrigins(genericListers configobserver.Listers, recorder events.Recorder, existingConfig map[string]interface{}) (map[string]interface{}, []error) { + return innerObserveAdditionalCORSAllowedOrigins(genericListers, recorder, existingConfig, []string{"corsAllowedOrigins"}) +} + +// ObserveAdditionalCORSAllowedOriginsToArguments observes the additionalCORSAllowedOrigins field +// of the APIServer resource and sets the cors-allowed-origins field in observedConfig.apiServerArguments +func ObserveAdditionalCORSAllowedOriginsToArguments(genericListers configobserver.Listers, recorder events.Recorder, existingConfig map[string]interface{}) (map[string]interface{}, []error) { + return innerObserveAdditionalCORSAllowedOrigins(genericListers, recorder, existingConfig, []string{"apiServerArguments", "cors-allowed-origins"}) +} + +func innerObserveAdditionalCORSAllowedOrigins(genericListers configobserver.Listers, recorder events.Recorder, existingConfig map[string]interface{}, corsAllowedOriginsPath []string) (ret map[string]interface{}, _ []error) { + defer func() { + ret = configobserver.Pruned(ret, corsAllowedOriginsPath) + }() + + lister := genericListers.(APIServerLister) + errs := []error{} + defaultConfig := map[string]interface{}{} + if err := unstructured.SetNestedStringSlice(defaultConfig, clusterDefaultCORSAllowedOrigins, corsAllowedOriginsPath...); err != nil { + // this should not happen + return existingConfig, append(errs, err) + } + + // grab the current CORS origins to later check whether they were updated + currentCORSAllowedOrigins, _, err := unstructured.NestedStringSlice(existingConfig, corsAllowedOriginsPath...) + if err != nil { + errs = append(errs, err) + // keep going on read error from existing config + } + currentCORSSet := sets.New(currentCORSAllowedOrigins...) + currentCORSSet.Insert(clusterDefaultCORSAllowedOrigins...) + + observedConfig := map[string]interface{}{} + apiServer, err := lister.APIServerLister().Get("cluster") + if errors.IsNotFound(err) { + klog.Warningf("apiserver.config.openshift.io/cluster: not found") + return defaultConfig, errs + } + if err != nil { + // return existingConfig here in case err is just a transient error so + // that we don't rewrite the config that was observed previously + return existingConfig, append(errs, err) + } + + newCORSSet := sets.New(clusterDefaultCORSAllowedOrigins...) + newCORSSet.Insert(apiServer.Spec.AdditionalCORSAllowedOrigins...) + if err := unstructured.SetNestedStringSlice(observedConfig, sets.List(newCORSSet), corsAllowedOriginsPath...); err != nil { + return existingConfig, append(errs, err) + } + + if !currentCORSSet.Equal(newCORSSet) { + recorder.Eventf("ObserveAdditionalCORSAllowedOrigins", "corsAllowedOrigins changed to %q", sets.List(newCORSSet)) + } + + return observedConfig, errs +} diff --git a/vendor/github.com/openshift/library-go/pkg/operator/configobserver/apiserver/observe_tlssecurityprofile.go b/vendor/github.com/openshift/library-go/pkg/operator/configobserver/apiserver/observe_tlssecurityprofile.go new file mode 100644 index 000000000..b04701681 --- /dev/null +++ b/vendor/github.com/openshift/library-go/pkg/operator/configobserver/apiserver/observe_tlssecurityprofile.go @@ -0,0 +1,109 @@ +package apiserver + +import ( + "fmt" + "reflect" + + "k8s.io/klog/v2" + + configv1 "github.com/openshift/api/config/v1" + "github.com/openshift/library-go/pkg/crypto" + "github.com/openshift/library-go/pkg/operator/configobserver" + "github.com/openshift/library-go/pkg/operator/events" + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" +) + +// ObserveTLSSecurityProfile observes APIServer.Spec.TLSSecurityProfile field and sets +// the ServingInfo.MinTLSVersion, ServingInfo.CipherSuites fields of observed config +func ObserveTLSSecurityProfile(genericListers configobserver.Listers, recorder events.Recorder, existingConfig map[string]interface{}) (map[string]interface{}, []error) { + return innerTLSSecurityProfileObservations(genericListers, recorder, existingConfig, []string{"servingInfo", "minTLSVersion"}, []string{"servingInfo", "cipherSuites"}) +} + +// ObserveTLSSecurityProfileWithPaths is like ObserveTLSSecurityProfile, but accepts +// custom paths for ServingInfo.MinTLSVersion and ServingInfo.CipherSuites fields of observed config. +func ObserveTLSSecurityProfileWithPaths(genericListers configobserver.Listers, recorder events.Recorder, existingConfig map[string]interface{}, minTLSVersionPath, cipherSuitesPath []string) (map[string]interface{}, []error) { + return innerTLSSecurityProfileObservations(genericListers, recorder, existingConfig, minTLSVersionPath, cipherSuitesPath) +} + +// ObserveTLSSecurityProfileToArguments observes APIServer.Spec.TLSSecurityProfile field and sets +// the tls-min-version and tls-cipher-suites fileds of observedConfig.apiServerArguments +func ObserveTLSSecurityProfileToArguments(genericListers configobserver.Listers, recorder events.Recorder, existingConfig map[string]interface{}) (map[string]interface{}, []error) { + return innerTLSSecurityProfileObservations(genericListers, recorder, existingConfig, []string{"apiServerArguments", "tls-min-version"}, []string{"apiServerArguments", "tls-cipher-suites"}) +} + +func innerTLSSecurityProfileObservations(genericListers configobserver.Listers, recorder events.Recorder, existingConfig map[string]interface{}, minTLSVersionPath, cipherSuitesPath []string) (ret map[string]interface{}, _ []error) { + defer func() { + ret = configobserver.Pruned(ret, minTLSVersionPath, cipherSuitesPath) + }() + + listers := genericListers.(APIServerLister) + errs := []error{} + + currentMinTLSVersion, _, versionErr := unstructured.NestedString(existingConfig, minTLSVersionPath...) + if versionErr != nil { + errs = append(errs, fmt.Errorf("failed to retrieve spec.servingInfo.minTLSVersion: %v", versionErr)) + // keep going on read error from existing config + } + + currentCipherSuites, _, suitesErr := unstructured.NestedStringSlice(existingConfig, cipherSuitesPath...) + if suitesErr != nil { + errs = append(errs, fmt.Errorf("failed to retrieve spec.servingInfo.cipherSuites: %v", suitesErr)) + // keep going on read error from existing config + } + + apiServer, err := listers.APIServerLister().Get("cluster") + if errors.IsNotFound(err) { + klog.Warningf("apiserver.config.openshift.io/cluster: not found") + apiServer = &configv1.APIServer{} + } else if err != nil { + return existingConfig, append(errs, err) + } + + observedConfig := map[string]interface{}{} + observedMinTLSVersion, observedCipherSuites := getSecurityProfileCiphers(apiServer.Spec.TLSSecurityProfile) + if err = unstructured.SetNestedField(observedConfig, observedMinTLSVersion, minTLSVersionPath...); err != nil { + return existingConfig, append(errs, err) + } + if err = unstructured.SetNestedStringSlice(observedConfig, observedCipherSuites, cipherSuitesPath...); err != nil { + return existingConfig, append(errs, err) + } + + if observedMinTLSVersion != currentMinTLSVersion { + recorder.Eventf("ObserveTLSSecurityProfile", "minTLSVersion changed to %s", observedMinTLSVersion) + } + if !reflect.DeepEqual(observedCipherSuites, currentCipherSuites) { + recorder.Eventf("ObserveTLSSecurityProfile", "cipherSuites changed to %q", observedCipherSuites) + } + + return observedConfig, errs +} + +// Extracts the minimum TLS version and cipher suites from TLSSecurityProfile object, +// Converts the ciphers to IANA names as supported by Kube ServingInfo config. +// If profile is nil, returns config defined by the Intermediate TLS Profile +func getSecurityProfileCiphers(profile *configv1.TLSSecurityProfile) (string, []string) { + var profileType configv1.TLSProfileType + if profile == nil { + profileType = configv1.TLSProfileIntermediateType + } else { + profileType = profile.Type + } + + var profileSpec *configv1.TLSProfileSpec + if profileType == configv1.TLSProfileCustomType { + if profile.Custom != nil { + profileSpec = &profile.Custom.TLSProfileSpec + } + } else { + profileSpec = configv1.TLSProfiles[profileType] + } + + // nothing found / custom type set but no actual custom spec + if profileSpec == nil { + profileSpec = configv1.TLSProfiles[configv1.TLSProfileIntermediateType] + } + + // need to remap all Ciphers to their respective IANA names used by Go + return string(profileSpec.MinTLSVersion), crypto.OpenSSLToIANACipherSuites(profileSpec.Ciphers) +} diff --git a/vendor/modules.txt b/vendor/modules.txt index c70c42a3a..38dea9f5f 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -467,6 +467,7 @@ github.com/openshift/library-go/pkg/network github.com/openshift/library-go/pkg/operator/certrotation github.com/openshift/library-go/pkg/operator/condition github.com/openshift/library-go/pkg/operator/configobserver +github.com/openshift/library-go/pkg/operator/configobserver/apiserver github.com/openshift/library-go/pkg/operator/configobserver/featuregates github.com/openshift/library-go/pkg/operator/deploymentcontroller github.com/openshift/library-go/pkg/operator/events