diff --git a/api/v1alpha1/meta.go b/api/v1alpha1/meta.go index 68a882d64..a54c4529d 100644 --- a/api/v1alpha1/meta.go +++ b/api/v1alpha1/meta.go @@ -111,8 +111,31 @@ type ProxyConfig struct { // +kubebuilder:validation:MaxLength:=4096 // +optional NoProxy string `json:"noProxy,omitempty"` + + // networkPolicyAllowProxyEgressAll controls whether the operator automatically creates a NetworkPolicy + // that allows ESO pods to reach the proxy server configured on the cluster. + // Managed (default): the operator creates and maintains the eso-sys-proxy-egress-core NetworkPolicy + // whenever an effective proxy is configured via getProxyConfiguration(). + // Unmanaged: the operator does not create this policy; administrators are responsible for + // defining their own egress rules for proxy traffic. + // This field has no effect on clusters without a proxy configuration. + // +kubebuilder:validation:Enum:=Managed;Unmanaged + // +kubebuilder:default:=Managed + // +optional + NetworkPolicyAllowProxyEgressAll NetworkPolicyManagementMode `json:"networkPolicyAllowProxyEgressAll,omitempty"` } +// NetworkPolicyManagementMode indicates whether the operator manages a NetworkPolicy automatically. +type NetworkPolicyManagementMode string + +const ( + // Managed indicates the operator creates and maintains the NetworkPolicy automatically. + Managed NetworkPolicyManagementMode = "Managed" + + // Unmanaged indicates the operator does not create the NetworkPolicy; the administrator manages it. + Unmanaged NetworkPolicyManagementMode = "Unmanaged" +) + // Mode indicates the operational state of the optional features. type Mode string diff --git a/api/v1alpha1/tests/externalsecretsconfig.operator.openshift.io/externalsecretsconfig.testsuite.yaml b/api/v1alpha1/tests/externalsecretsconfig.operator.openshift.io/externalsecretsconfig.testsuite.yaml index 6e20a96df..6ef439623 100644 --- a/api/v1alpha1/tests/externalsecretsconfig.operator.openshift.io/externalsecretsconfig.testsuite.yaml +++ b/api/v1alpha1/tests/externalsecretsconfig.operator.openshift.io/externalsecretsconfig.testsuite.yaml @@ -1445,6 +1445,96 @@ tests: injectAnnotations: "false" certificateDuration: "8760h" certificateRenewBefore: "30m" + - name: Should accept networkPolicyAllowProxyEgressAll set to Managed (default) + resourceName: cluster + initial: | + apiVersion: operator.openshift.io/v1alpha1 + kind: ExternalSecretsConfig + spec: + appConfig: + proxy: + httpProxy: "http://proxy.example.com:3128" + networkPolicyAllowProxyEgressAll: Managed + expected: | + apiVersion: operator.openshift.io/v1alpha1 + kind: ExternalSecretsConfig + spec: + appConfig: + logLevel: 1 + proxy: + httpProxy: "http://proxy.example.com:3128" + networkPolicyAllowProxyEgressAll: Managed + - name: Should accept networkPolicyAllowProxyEgressAll set to Unmanaged + resourceName: cluster + initial: | + apiVersion: operator.openshift.io/v1alpha1 + kind: ExternalSecretsConfig + spec: + appConfig: + proxy: + httpProxy: "http://proxy.example.com:3128" + networkPolicyAllowProxyEgressAll: Unmanaged + expected: | + apiVersion: operator.openshift.io/v1alpha1 + kind: ExternalSecretsConfig + spec: + appConfig: + logLevel: 1 + proxy: + httpProxy: "http://proxy.example.com:3128" + networkPolicyAllowProxyEgressAll: Unmanaged + - name: Should default networkPolicyAllowProxyEgressAll to Managed when proxy is set without the field + resourceName: cluster + initial: | + apiVersion: operator.openshift.io/v1alpha1 + kind: ExternalSecretsConfig + spec: + appConfig: + proxy: + httpProxy: "http://proxy.example.com:3128" + expected: | + apiVersion: operator.openshift.io/v1alpha1 + kind: ExternalSecretsConfig + spec: + appConfig: + logLevel: 1 + proxy: + httpProxy: "http://proxy.example.com:3128" + networkPolicyAllowProxyEgressAll: Managed + - name: Should fail with invalid value for networkPolicyAllowProxyEgressAll + resourceName: cluster + initial: | + apiVersion: operator.openshift.io/v1alpha1 + kind: ExternalSecretsConfig + spec: + appConfig: + proxy: + httpProxy: "http://proxy.example.com:3128" + networkPolicyAllowProxyEgressAll: Enabled + expectedError: "spec.appConfig.proxy.networkPolicyAllowProxyEgressAll: Unsupported value: \"Enabled\": supported values: \"Managed\", \"Unmanaged\"" + - name: Should accept proxy config with all fields including networkPolicyAllowProxyEgressAll + resourceName: cluster + initial: | + apiVersion: operator.openshift.io/v1alpha1 + kind: ExternalSecretsConfig + spec: + appConfig: + proxy: + httpProxy: "http://proxy.example.com:3128" + httpsProxy: "https://proxy.example.com:3128" + noProxy: "localhost,127.0.0.1,.cluster.local" + networkPolicyAllowProxyEgressAll: Unmanaged + expected: | + apiVersion: operator.openshift.io/v1alpha1 + kind: ExternalSecretsConfig + spec: + appConfig: + logLevel: 1 + proxy: + httpProxy: "http://proxy.example.com:3128" + httpsProxy: "https://proxy.example.com:3128" + noProxy: "localhost,127.0.0.1,.cluster.local" + networkPolicyAllowProxyEgressAll: Unmanaged onUpdate: - name: Should be able to update labels in controller config resourceName: cluster @@ -1726,3 +1816,30 @@ tests: - to: - ipBlock: cidr: 172.16.0.0/12 + - name: Should be able to update networkPolicyAllowProxyEgressAll from Managed to Unmanaged + resourceName: cluster + initial: | + apiVersion: operator.openshift.io/v1alpha1 + kind: ExternalSecretsConfig + spec: + appConfig: + proxy: + httpProxy: "http://proxy.example.com:3128" + networkPolicyAllowProxyEgressAll: Managed + updated: | + apiVersion: operator.openshift.io/v1alpha1 + kind: ExternalSecretsConfig + spec: + appConfig: + proxy: + httpProxy: "http://proxy.example.com:3128" + networkPolicyAllowProxyEgressAll: Unmanaged + expected: | + apiVersion: operator.openshift.io/v1alpha1 + kind: ExternalSecretsConfig + spec: + appConfig: + logLevel: 1 + proxy: + httpProxy: "http://proxy.example.com:3128" + networkPolicyAllowProxyEgressAll: Unmanaged diff --git a/api/v1alpha1/tests/externalsecretsmanager.operator.openshift.io/externalsecretsmanager.testsuite.yaml b/api/v1alpha1/tests/externalsecretsmanager.operator.openshift.io/externalsecretsmanager.testsuite.yaml index 2f88c646a..f26ff87e2 100644 --- a/api/v1alpha1/tests/externalsecretsmanager.operator.openshift.io/externalsecretsmanager.testsuite.yaml +++ b/api/v1alpha1/tests/externalsecretsmanager.operator.openshift.io/externalsecretsmanager.testsuite.yaml @@ -89,6 +89,7 @@ tests: httpProxy: "http://proxy.example.com:8080" httpsProxy: "https://proxy.example.com:8443" noProxy: "localhost,127.0.0.1,.local" + networkPolicyAllowProxyEgressAll: Managed - name: Should fail to create with invalid singleton name resourceName: invalid-name initial: | @@ -182,6 +183,7 @@ tests: logLevel: 1 proxy: httpProxy: "http://proxy-url-at-exactly-two-thousand-and-forty-eight-characters-to-test-the-boundary-condition-where-we-want-to-ensure-that-urls-at-the-maximum-allowed-length-are-accepted-properly-by-the-validation-system-while-urls-that-exceed-this-limit-are-rejected-appropriately-which-is-important-for-maintaining-proper-validation-boundaries-in-production-systems-where-configuration-parameters-must-be-validated-correctly-to-prevent-system-failures-or-unexpected-behavior-that-could-impact-application-functionality-and-user-experience-in-various-deployment-environments-including-development-staging-and-production-kubernetes-clusters-running-across-different-cloud-providers-and-on-premises-infrastructure-where-proxy-configurations-are-commonly-used-for-network-security-and-compliance-requirements-that-organizations-need-to-meet-for-their-business-operations-and-regulatory-obligations-in-different-geographical-regions-around-the-world-where-various-network-policies-and-security-measures-are-implemented-to-protect-sensitive-data-and-ensure-proper-access-control-for-applications-and-services-that-handle-confidential-information-and-business-critical-processes-that-must-operate-reliably-and-securely-at-all-times-without-interruption-or-performance-degradation-that-could-affect-end-users-and-customers-who-depend-on-these-systems-for-their-daily-activities-and-business-needs-which-makes-proper-validation-of-configuration-parameters-like-proxy-urls-essential-for-maintaining-system-stability-and-security-in-production-environments-where-any-configuration-error-could-have-significant-consequences-for-business-continuity-and-customer-satisfaction-which-is-why-we-implement-comprehensive-boundary-testing-to-ensure-that-all-validation-rules-work-correctly-at-their-specified-limits-and-provide-clear-error-messages-when-those-limits-are-exceeded-by-user-configurations.example.com:8080" + networkPolicyAllowProxyEgressAll: Managed - name: Should accept HTTPS proxy URL at maximum length boundary resourceName: cluster initial: | @@ -200,6 +202,7 @@ tests: logLevel: 1 proxy: httpsProxy: "https://secure-proxy-url-at-exactly-two-thousand-and-forty-eight-characters-to-test-the-boundary-condition-where-we-want-to-ensure-that-urls-at-the-maximum-allowed-length-are-accepted-properly-by-the-validation-system-while-urls-that-exceed-this-limit-are-rejected-appropriately-which-is-important-for-maintaining-proper-validation-boundaries-in-production-systems-where-configuration-parameters-must-be-validated-correctly-to-prevent-system-failures-or-unexpected-behavior-that-could-impact-application-functionality-and-user-experience-in-various-deployment-environments-including-development-staging-and-production-kubernetes-clusters-running-across-different-cloud-providers-and-on-premises-infrastructure-where-proxy-configurations-are-commonly-used-for-network-security-and-compliance-requirements-that-organizations-need-to-meet-for-their-business-operations-and-regulatory-obligations-in-different-geographical-regions-around-the-world-where-various-network-policies-and-security-measures-are-implemented-to-protect-sensitive-data-and-ensure-proper-access-control-for-applications-and-services-that-handle-confidential-information-and-business-critical-processes-that-must-operate-reliably-and-securely-at-all-times-without-interruption-or-performance-degradation-that-could-affect-end-users-and-customers-who-depend-on-these-systems-for-their-daily-activities-and-business-needs-which-makes-proper-validation-of-configuration-parameters-like-proxy-urls-essential-for-maintaining-system-stability-and-security-in-production-environments-where-any-configuration-error-could-have-significant-consequences-for-business-continuity-and-customer-satisfaction-which-is-why-we-implement-comprehensive-boundary-testing-to-ensure-that-all-validation-rules-work-correctly-at-their-specified-limits-and-provide-clear-error-messages-when-those-limits-are-exceeded.example.com:8443" + networkPolicyAllowProxyEgressAll: Managed - name: Should accept empty proxy configuration resourceName: cluster initial: | @@ -221,6 +224,7 @@ tests: httpProxy: "" httpsProxy: "" noProxy: "" + networkPolicyAllowProxyEgressAll: Managed onUpdate: - name: Should be able to add global config after creation resourceName: cluster @@ -338,6 +342,7 @@ tests: httpProxy: "http://proxy.company.com:3128" httpsProxy: "https://proxy.company.com:3128" noProxy: "localhost,127.0.0.1,.company.com" + networkPolicyAllowProxyEgressAll: Managed - name: Should be able to update node selector and tolerations resourceName: cluster initial: | diff --git a/bundle/manifests/openshift-external-secrets-operator.clusterserviceversion.yaml b/bundle/manifests/openshift-external-secrets-operator.clusterserviceversion.yaml index a87455abc..5339860ba 100644 --- a/bundle/manifests/openshift-external-secrets-operator.clusterserviceversion.yaml +++ b/bundle/manifests/openshift-external-secrets-operator.clusterserviceversion.yaml @@ -605,6 +605,7 @@ spec: - networkpolicies verbs: - create + - delete - get - list - update diff --git a/config/crd/bases/operator.openshift.io_externalsecretsconfigs.yaml b/config/crd/bases/operator.openshift.io_externalsecretsconfigs.yaml index 274ac9416..db30f444f 100644 --- a/config/crd/bases/operator.openshift.io_externalsecretsconfigs.yaml +++ b/config/crd/bases/operator.openshift.io_externalsecretsconfigs.yaml @@ -1034,6 +1034,20 @@ spec: maxLength: 2048 minLength: 0 type: string + networkPolicyAllowProxyEgressAll: + description: |- + networkPolicyAllowProxyEgressAll controls whether the operator automatically creates a NetworkPolicy + that allows ESO pods to reach the proxy server configured on the cluster. + Managed (default): the operator creates and maintains the eso-sys-proxy-egress-core NetworkPolicy + whenever an effective proxy is configured via getProxyConfiguration(). + Unmanaged: the operator does not create this policy; administrators are responsible for + defining their own egress rules for proxy traffic. + This field has no effect on clusters without a proxy configuration. + default: Managed + enum: + - Managed + - Unmanaged + type: string noProxy: description: |- noProxy is a comma-separated list of hostnames and/or CIDRs and/or IPs for which the proxy should not be used. diff --git a/config/crd/bases/operator.openshift.io_externalsecretsmanagers.yaml b/config/crd/bases/operator.openshift.io_externalsecretsmanagers.yaml index c508693b4..cc1c7d5a1 100644 --- a/config/crd/bases/operator.openshift.io_externalsecretsmanagers.yaml +++ b/config/crd/bases/operator.openshift.io_externalsecretsmanagers.yaml @@ -1031,6 +1031,20 @@ spec: maxLength: 2048 minLength: 0 type: string + networkPolicyAllowProxyEgressAll: + description: |- + networkPolicyAllowProxyEgressAll controls whether the operator automatically creates a NetworkPolicy + that allows ESO pods to reach the proxy server configured on the cluster. + Managed (default): the operator creates and maintains the eso-sys-proxy-egress-core NetworkPolicy + whenever an effective proxy is configured via getProxyConfiguration(). + Unmanaged: the operator does not create this policy; administrators are responsible for + defining their own egress rules for proxy traffic. + This field has no effect on clusters without a proxy configuration. + default: Managed + enum: + - Managed + - Unmanaged + type: string noProxy: description: |- noProxy is a comma-separated list of hostnames and/or CIDRs and/or IPs for which the proxy should not be used. diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 988aaefcb..e6302d975 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -181,6 +181,7 @@ rules: - networkpolicies verbs: - create + - delete - get - list - update diff --git a/pkg/controller/external_secrets/constants.go b/pkg/controller/external_secrets/constants.go index 8850ed83a..940eff6c4 100644 --- a/pkg/controller/external_secrets/constants.go +++ b/pkg/controller/external_secrets/constants.go @@ -66,6 +66,19 @@ const ( // https://cs.opensource.google/go/go/+/refs/tags/go1.24.4:src/crypto/x509/root_linux.go;l=22 trustedCABundleMountPath = "/etc/pki/tls/certs" + // proxyEgressNetworkPolicyName is the name of the programmatically-built proxy egress NetworkPolicy. + // Created only when spec.appConfig.proxy.networkPolicyAllowProxyEgressAll is Managed (default) + // and an effective proxy is configured via getProxyConfiguration(). + proxyEgressNetworkPolicyName = "eso-sys-proxy-egress-core" + + // migrationCompleteAnnotation is set on ExternalSecretsConfig after legacy unprefixed + // NetworkPolicies have been pruned on the first reconcile under the new naming scheme. + migrationCompleteAnnotation = "operator.openshift.io/network-policy-migration-complete" + + // userNetworkPolicyPrefix is prepended to user-defined NetworkPolicy names from spec.networkPolicies[]. + // The user writes name: allow-external-secrets-egress; the K8s object is eso-user-allow-external-secrets-egress. + userNetworkPolicyPrefix = "eso-user-" + // Proxy environment variable names (uppercase). httpProxyEnvVar = "HTTP_PROXY" httpsProxyEnvVar = "HTTPS_PROXY" diff --git a/pkg/controller/external_secrets/controller.go b/pkg/controller/external_secrets/controller.go index 030566d5e..49035797c 100644 --- a/pkg/controller/external_secrets/controller.go +++ b/pkg/controller/external_secrets/controller.go @@ -104,7 +104,7 @@ type Reconciler struct { // +kubebuilder:rbac:groups="",resources=events;secrets;services;serviceaccounts,verbs=get;list;watch;create;update;delete;patch // +kubebuilder:rbac:groups=apps,resources=deployments,verbs=get;list;watch;create;update // +kubebuilder:rbac:groups=cert-manager.io,resources=certificates;clusterissuers;issuers,verbs=get;list;watch;create;update -// +kubebuilder:rbac:groups=networking.k8s.io,resources=networkpolicies,verbs=get;list;watch;create;update +// +kubebuilder:rbac:groups=networking.k8s.io,resources=networkpolicies,verbs=get;list;watch;create;update;delete // +kubebuilder:rbac:groups="",resources=namespaces,verbs=get;list;watch;create;update;patch // +kubebuilder:rbac:groups="",resources=endpoints,verbs=get;list;watch diff --git a/pkg/controller/external_secrets/networkpolicy.go b/pkg/controller/external_secrets/networkpolicy.go index 622625cfd..49ab3940a 100644 --- a/pkg/controller/external_secrets/networkpolicy.go +++ b/pkg/controller/external_secrets/networkpolicy.go @@ -1,11 +1,16 @@ package external_secrets import ( + "encoding/json" "fmt" + "net/url" + "strconv" corev1 "k8s.io/api/core/v1" networkingv1 "k8s.io/api/networking/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/intstr" "sigs.k8s.io/controller-runtime/pkg/client" operatorv1alpha1 "github.com/openshift/external-secrets-operator/api/v1alpha1" @@ -16,16 +21,22 @@ import ( // createOrApplyNetworkPolicies handles creation of both static network policies from manifests // and custom network policies configured in the ExternalSecretsConfig API. func (r *Reconciler) createOrApplyNetworkPolicies(esc *operatorv1alpha1.ExternalSecretsConfig, resourceMetadata common.ResourceMetadata, externalSecretsConfigCreateRecon bool) error { - // First, apply the static deny-all network policy if err := r.createOrApplyStaticNetworkPolicies(esc, resourceMetadata, externalSecretsConfigCreateRecon); err != nil { return err } - // Then, apply custom network policies from the API spec if err := r.createOrApplyCustomNetworkPolicies(esc, resourceMetadata, externalSecretsConfigCreateRecon); err != nil { return err } + if err := r.createOrApplyProxyEgressNetworkPolicy(esc, resourceMetadata, externalSecretsConfigCreateRecon); err != nil { + return err + } + + if err := r.cleanupMigratedNetworkPolicies(esc, resourceMetadata); err != nil { + return err + } + return nil } @@ -50,7 +61,7 @@ func (r *Reconciler) createOrApplyStaticNetworkPolicies(esc *operatorv1alpha1.Ex }, { assetName: allowCertControllerTrafficAssetName, - condition: !isCertManagerConfigEnabled(esc), // Only if cert-controller is enabled + condition: !isCertManagerConfigEnabled(esc), // Only if cert-controller is needed (cert-manager replaces it) }, { assetName: allowBitwardenServerTrafficAssetName, @@ -172,19 +183,19 @@ func (r *Reconciler) createOrApplyNetworkPolicyFromAsset(esc *operatorv1alpha1.E } // buildNetworkPolicyFromConfig constructs a NetworkPolicy object from the API configuration. +// The resulting K8s object name is prefixed with userNetworkPolicyPrefix (eso-user-) so that +// operator-owned user policies are unambiguously identified and prunable by cleanupMigratedNetworkPolicies. func (r *Reconciler) buildNetworkPolicyFromConfig(esc *operatorv1alpha1.ExternalSecretsConfig, npConfig operatorv1alpha1.NetworkPolicy, resourceMetadata common.ResourceMetadata) (*networkingv1.NetworkPolicy, error) { namespace := getNamespace(esc) - // Determine pod selector based on component name podSelector, err := r.getPodSelectorForComponent(npConfig.ComponentName) if err != nil { return nil, fmt.Errorf("failed to determine pod selector for network policy %s: %w", npConfig.Name, err) } - // Build the NetworkPolicy object networkPolicy := &networkingv1.NetworkPolicy{ ObjectMeta: metav1.ObjectMeta{ - Name: npConfig.Name, + Name: userNetworkPolicyPrefix + npConfig.Name, Namespace: namespace, }, Spec: networkingv1.NetworkPolicySpec{ @@ -218,3 +229,265 @@ func (r *Reconciler) getPodSelectorForComponent(componentName operatorv1alpha1.C return metav1.LabelSelector{}, fmt.Errorf("unknown component name: %s", componentName) } } + +// createOrApplyProxyEgressNetworkPolicy manages the eso-sys-proxy-egress-core NetworkPolicy. +// It is created when spec.appConfig.proxy.networkPolicyAllowProxyEgressAll is Managed (default) +// AND an effective proxy is configured. When either condition is not met, any existing policy +// is deleted to avoid stale allow rules. +func (r *Reconciler) createOrApplyProxyEgressNetworkPolicy(esc *operatorv1alpha1.ExternalSecretsConfig, resourceMetadata common.ResourceMetadata, externalSecretsConfigCreateRecon bool) error { + proxyConfig := r.getProxyConfiguration(esc) + namespace := getNamespace(esc) + npName := fmt.Sprintf("%s/%s", namespace, proxyEgressNetworkPolicyName) + + if !shouldManageProxyEgress(esc, proxyConfig) { + fetched := &networkingv1.NetworkPolicy{} + exists, err := r.Exists(r.ctx, types.NamespacedName{Name: proxyEgressNetworkPolicyName, Namespace: namespace}, fetched) + if err != nil { + return common.FromClientError(err, "failed to check existence of proxy egress network policy %s", npName) + } + if exists { + r.log.V(1).Info("Proxy egress policy no longer needed, deleting", "name", npName) + if err := r.Delete(r.ctx, fetched); err != nil { + return common.FromClientError(err, "failed to delete proxy egress network policy %s", npName) + } + r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "NetworkPolicy %s deleted", npName) + } + return nil + } + + networkPolicy := r.buildProxyEgressNetworkPolicy(esc, proxyConfig, resourceMetadata) + r.log.V(4).Info("Reconciling proxy egress network policy", "name", npName) + + fetched := &networkingv1.NetworkPolicy{} + exists, err := r.Exists(r.ctx, client.ObjectKeyFromObject(networkPolicy), fetched) + if err != nil { + return common.FromClientError(err, "failed to check existence of proxy egress network policy %s", npName) + } + + if exists && externalSecretsConfigCreateRecon { + r.eventRecorder.Eventf(esc, corev1.EventTypeWarning, "ResourceAlreadyExists", "NetworkPolicy %s already exists", npName) + } + + switch { + case exists && common.HasObjectChanged(networkPolicy, fetched, &resourceMetadata): + r.log.V(1).Info("Proxy egress NetworkPolicy modified, updating", "name", npName) + common.RemoveObsoleteAnnotations(networkPolicy, resourceMetadata) + if err := r.UpdateWithRetry(r.ctx, networkPolicy); err != nil { + return common.FromClientError(err, "failed to update proxy egress network policy %s", npName) + } + r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "NetworkPolicy %s updated", npName) + case !exists: + if err := r.Create(r.ctx, networkPolicy); err != nil { + return common.FromClientError(err, "failed to create proxy egress network policy %s", npName) + } + r.eventRecorder.Eventf(esc, corev1.EventTypeNormal, "Reconciled", "NetworkPolicy %s created", npName) + default: + r.log.V(4).Info("Proxy egress NetworkPolicy already up-to-date", "name", npName) + } + + return nil +} + +// buildProxyEgressNetworkPolicy constructs the eso-sys-proxy-egress-core NetworkPolicy. +// The proxy port is extracted at reconcile time from the effective proxy configuration. +func (r *Reconciler) buildProxyEgressNetworkPolicy(esc *operatorv1alpha1.ExternalSecretsConfig, proxyConfig *operatorv1alpha1.ProxyConfig, resourceMetadata common.ResourceMetadata) *networkingv1.NetworkPolicy { + port := intstr.FromInt32(getProxyPort(proxyConfig)) + protocol := corev1.ProtocolTCP + + np := &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: proxyEgressNetworkPolicyName, + Namespace: getNamespace(esc), + }, + Spec: networkingv1.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app.kubernetes.io/name": externalsecretsCommonName, + }, + }, + PolicyTypes: []networkingv1.PolicyType{networkingv1.PolicyTypeEgress}, + Egress: []networkingv1.NetworkPolicyEgressRule{ + { + Ports: []networkingv1.NetworkPolicyPort{ + {Protocol: &protocol, Port: &port}, + }, + }, + }, + }, + } + common.ApplyResourceMetadata(np, resourceMetadata) + return np +} + +// shouldManageProxyEgress returns true when the operator should create the proxy egress NetworkPolicy. +// Both conditions must hold: an effective proxy is configured AND the management mode is Managed (default). +func shouldManageProxyEgress(esc *operatorv1alpha1.ExternalSecretsConfig, proxyConfig *operatorv1alpha1.ProxyConfig) bool { + if proxyConfig == nil { + return false + } + if esc.Spec.ApplicationConfig.Proxy == nil { + // Proxy came from ESM or OLM env vars; default management mode is Managed. + return true + } + mode := esc.Spec.ApplicationConfig.Proxy.NetworkPolicyAllowProxyEgressAll + return mode == "" || mode == operatorv1alpha1.Managed +} + +// getProxyPort extracts the TCP port from the proxy URL. +// It tries HTTPSProxy first, then HTTPProxy. Falls back to 3128 (common squid default). +func getProxyPort(proxyConfig *operatorv1alpha1.ProxyConfig) int32 { + for _, rawURL := range []string{proxyConfig.HTTPSProxy, proxyConfig.HTTPProxy} { + if rawURL == "" { + continue + } + u, err := url.Parse(rawURL) + if err != nil { + continue + } + portStr := u.Port() + if portStr != "" { + port, err := strconv.ParseInt(portStr, 10, 32) + if err == nil && port > 0 { + return int32(port) + } + } + switch u.Scheme { + case "https": + return 443 + case "http": + return 80 + } + } + return 3128 +} + +// cleanupMigratedNetworkPolicies prunes stale operator-managed NetworkPolicies from the operand namespace. +// On every reconcile it lists all NPs with the operator's managed-by label and deletes any whose name +// is not in the current desired set (catches entries removed from the CR spec). +// On the first reconcile after upgrade (no migrationCompleteAnnotation on the CR), it also deletes any +// legacy NetworkPolicies listed without the label. Once cleanup succeeds, the annotation is set. +func (r *Reconciler) cleanupMigratedNetworkPolicies(esc *operatorv1alpha1.ExternalSecretsConfig, resourceMetadata common.ResourceMetadata) error { + namespace := getNamespace(esc) + desired := r.desiredNetworkPolicyNames(esc, r.getProxyConfiguration(esc)) + + // Always: label-based list to catch stale entries. + labeledList := &networkingv1.NetworkPolicyList{} + if err := r.List(r.ctx, labeledList, + client.InNamespace(namespace), + client.MatchingLabels{"app.kubernetes.io/managed-by": common.ExternalSecretsOperatorCommonName}, + ); err != nil { + return common.FromClientError(err, "failed to list network policies in namespace %s", namespace) + } + + for i := range labeledList.Items { + np := &labeledList.Items[i] + if _, ok := desired[np.Name]; !ok { + r.log.V(1).Info("Pruning stale network policy", "name", np.Name, "namespace", namespace) + if err := r.Delete(r.ctx, np); err != nil { + return common.FromClientError(err, "failed to delete stale network policy %s/%s", namespace, np.Name) + } + } + } + + // First-reconcile migration: clean up legacy unprefixed names that lacked the managed-by label. + if _, hasMigration := esc.GetAnnotations()[migrationCompleteAnnotation]; !hasMigration { + if err := r.deleteLegacyNetworkPolicies(namespace, desired); err != nil { + return err + } + if err := r.setMigrationCompleteAnnotation(esc); err != nil { + return err + } + } + + return nil +} + +// desiredNetworkPolicyNames returns the set of NetworkPolicy names currently managed by the operator. +func (r *Reconciler) desiredNetworkPolicyNames(esc *operatorv1alpha1.ExternalSecretsConfig, proxyConfig *operatorv1alpha1.ProxyConfig) map[string]struct{} { + names := make(map[string]struct{}) + + // Static bindata-based NPs. + for name := range staticNetworkPolicyNames(esc) { + names[name] = struct{}{} + } + + // Proxy egress NP. + if shouldManageProxyEgress(esc, proxyConfig) { + names[proxyEgressNetworkPolicyName] = struct{}{} + } + + // User-defined NPs (with eso-user- prefix). + for _, np := range esc.Spec.ControllerConfig.NetworkPolicies { + names[userNetworkPolicyPrefix+np.Name] = struct{}{} + } + + return names +} + +// staticNetworkPolicyNames returns the set of NP names created from bindata assets for this ESC. +func staticNetworkPolicyNames(esc *operatorv1alpha1.ExternalSecretsConfig) map[string]struct{} { + names := map[string]struct{}{ + "deny-all-traffic": {}, + "allow-api-server-egress-for-main-controller": {}, + "allow-api-server-egress-for-webhook": {}, + "allow-to-dns": {}, + } + if !isCertManagerConfigEnabled(esc) { + names["allow-api-server-egress-for-cert-controller"] = struct{}{} + } + if isBitwardenConfigEnabled(esc) { + names["allow-api-server-egress-for-bitwarden-server"] = struct{}{} + } + return names +} + +// deleteLegacyNetworkPolicies removes legacy NetworkPolicies by name that predate the eso-user- prefix scheme. +// These NPs were created without the managed-by label so they won't appear in the label-based list. +func (r *Reconciler) deleteLegacyNetworkPolicies(namespace string, desired map[string]struct{}) error { + legacyNames := []string{ + "deny-all-traffic", + "allow-api-server-egress", + "allow-api-server-egress-for-webhook", + "allow-api-server-egress-for-cert-controller", + "allow-api-server-egress-for-bitwarden-sever", + "allow-to-dns", + } + for _, name := range legacyNames { + if _, isDesired := desired[name]; isDesired { + continue + } + np := &networkingv1.NetworkPolicy{} + exists, err := r.Exists(r.ctx, types.NamespacedName{Name: name, Namespace: namespace}, np) + if err != nil { + return common.FromClientError(err, "failed to check legacy network policy %s/%s", namespace, name) + } + if exists { + r.log.V(1).Info("Deleting legacy network policy", "name", name, "namespace", namespace) + if err := r.Delete(r.ctx, np); err != nil { + return common.FromClientError(err, "failed to delete legacy network policy %s/%s", namespace, name) + } + } + } + return nil +} + +// setMigrationCompleteAnnotation patches the migrationCompleteAnnotation onto the ExternalSecretsConfig CR. +func (r *Reconciler) setMigrationCompleteAnnotation(esc *operatorv1alpha1.ExternalSecretsConfig) error { + patchBody := map[string]interface{}{ + "metadata": map[string]interface{}{ + "annotations": map[string]string{ + migrationCompleteAnnotation: "true", + }, + }, + } + patchBytes, err := json.Marshal(patchBody) + if err != nil { + return fmt.Errorf("failed to marshal migration annotation patch: %w", err) + } + patch := client.RawPatch(types.MergePatchType, patchBytes) + if err := r.Patch(r.ctx, esc, patch, client.FieldOwner(common.ExternalSecretsOperatorCommonName)); err != nil { + return common.FromClientError(err, "failed to set migration complete annotation on %s", esc.GetName()) + } + r.log.V(4).Info("Network policy migration annotation set", "name", esc.GetName()) + return nil +} diff --git a/pkg/controller/external_secrets/networkpolicy_test.go b/pkg/controller/external_secrets/networkpolicy_test.go index 6eb45e097..1f7977158 100644 --- a/pkg/controller/external_secrets/networkpolicy_test.go +++ b/pkg/controller/external_secrets/networkpolicy_test.go @@ -277,7 +277,7 @@ func TestCreateOrApplyCustomNetworkPolicies(t *testing.T) { }) m.CreateCalls(func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { if np, ok := obj.(*networkingv1.NetworkPolicy); ok { - if np.Name != "test-custom-policy" { + if np.Name != userNetworkPolicyPrefix+"test-custom-policy" { return fmt.Errorf("unexpected network policy name: %s", np.Name) } } @@ -343,7 +343,7 @@ func TestCreateOrApplyCustomNetworkPolicies(t *testing.T) { }, } }, - wantErr: "failed to create network policy external-secrets/test-fail-policy: test client error", + wantErr: "failed to create network policy external-secrets/" + userNetworkPolicyPrefix + "test-fail-policy: test client error", }, { name: "custom network policy updated successfully", @@ -352,7 +352,7 @@ func TestCreateOrApplyCustomNetworkPolicies(t *testing.T) { if o, ok := obj.(*networkingv1.NetworkPolicy); ok { np := &networkingv1.NetworkPolicy{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-update-policy", + Name: userNetworkPolicyPrefix + "test-update-policy", Namespace: externalsecretsDefaultNamespace, }, } @@ -494,7 +494,7 @@ func TestBuildNetworkPolicyFromConfig(t *testing.T) { }, wantErr: false, wantPolicy: func(np *networkingv1.NetworkPolicy) bool { - return np.Name == "test-core-policy" && + return np.Name == userNetworkPolicyPrefix+"test-core-policy" && np.Spec.PodSelector.MatchLabels["app.kubernetes.io/name"] == externalsecretsCommonName && len(np.Spec.Egress) == 1 && len(np.Spec.PolicyTypes) == 1 && @@ -510,7 +510,7 @@ func TestBuildNetworkPolicyFromConfig(t *testing.T) { }, wantErr: false, wantPolicy: func(np *networkingv1.NetworkPolicy) bool { - return np.Name == "test-bitwarden-policy" && + return np.Name == userNetworkPolicyPrefix+"test-bitwarden-policy" && np.Spec.PodSelector.MatchLabels["app.kubernetes.io/name"] == bitwardenSDKServerContainerName }, }, @@ -547,3 +547,387 @@ func TestBuildNetworkPolicyFromConfig(t *testing.T) { }) } } + +func TestShouldManageProxyEgress(t *testing.T) { + tests := []struct { + name string + proxyConfig *operatorv1alpha1.ProxyConfig + escProxy *operatorv1alpha1.ProxyConfig + want bool + }{ + { + name: "no proxy configured", + proxyConfig: nil, + want: false, + }, + { + name: "proxy from ESM or OLM, ESC proxy nil defaults to Managed", + proxyConfig: &operatorv1alpha1.ProxyConfig{HTTPSProxy: "https://proxy.example.com:8080"}, + escProxy: nil, + want: true, + }, + { + name: "explicit Managed mode", + proxyConfig: &operatorv1alpha1.ProxyConfig{HTTPSProxy: "https://proxy.example.com:8080"}, + escProxy: &operatorv1alpha1.ProxyConfig{ + HTTPSProxy: "https://proxy.example.com:8080", + NetworkPolicyAllowProxyEgressAll: operatorv1alpha1.Managed, + }, + want: true, + }, + { + name: "Unmanaged mode", + proxyConfig: &operatorv1alpha1.ProxyConfig{HTTPSProxy: "https://proxy.example.com:8080"}, + escProxy: &operatorv1alpha1.ProxyConfig{ + HTTPSProxy: "https://proxy.example.com:8080", + NetworkPolicyAllowProxyEgressAll: operatorv1alpha1.Unmanaged, + }, + want: false, + }, + { + name: "empty mode field defaults to Managed", + proxyConfig: &operatorv1alpha1.ProxyConfig{HTTPSProxy: "https://proxy.example.com:8080"}, + escProxy: &operatorv1alpha1.ProxyConfig{ + HTTPSProxy: "https://proxy.example.com:8080", + }, + want: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + esc := commontest.TestExternalSecretsConfig() + esc.Spec.ApplicationConfig.Proxy = tt.escProxy + got := shouldManageProxyEgress(esc, tt.proxyConfig) + if got != tt.want { + t.Errorf("shouldManageProxyEgress() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestGetProxyPort(t *testing.T) { + tests := []struct { + name string + proxyConfig *operatorv1alpha1.ProxyConfig + want int32 + }{ + { + name: "HTTPS proxy with explicit port", + proxyConfig: &operatorv1alpha1.ProxyConfig{HTTPSProxy: "https://proxy.example.com:8080"}, + want: 8080, + }, + { + name: "HTTP proxy with explicit port", + proxyConfig: &operatorv1alpha1.ProxyConfig{HTTPProxy: "http://proxy.example.com:3128"}, + want: 3128, + }, + { + name: "HTTPS proxy without port uses scheme default 443", + proxyConfig: &operatorv1alpha1.ProxyConfig{HTTPSProxy: "https://proxy.example.com"}, + want: 443, + }, + { + name: "HTTP proxy without port uses scheme default 80", + proxyConfig: &operatorv1alpha1.ProxyConfig{HTTPProxy: "http://proxy.example.com"}, + want: 80, + }, + { + name: "HTTPS proxy takes priority over HTTP proxy", + proxyConfig: &operatorv1alpha1.ProxyConfig{ + HTTPSProxy: "https://proxy.example.com:9443", + HTTPProxy: "http://proxy.example.com:8080", + }, + want: 9443, + }, + { + name: "no proxy URLs falls back to default port 3128", + proxyConfig: &operatorv1alpha1.ProxyConfig{}, + want: 3128, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := getProxyPort(tt.proxyConfig) + if got != tt.want { + t.Errorf("getProxyPort() = %d, want %d", got, tt.want) + } + }) + } +} + +func TestCreateOrApplyProxyEgressNetworkPolicy(t *testing.T) { + proxyEgressNPName := fmt.Sprintf("%s/%s", externalsecretsDefaultNamespace, proxyEgressNetworkPolicyName) + + tests := []struct { + name string + preReq func(*Reconciler, *fakes.FakeCtrlClient) + updateExternalSecretsConfig func(*operatorv1alpha1.ExternalSecretsConfig) + wantErr string + }{ + { + name: "no proxy configured, no NP created or deleted", + 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 { + if np, ok := obj.(*networkingv1.NetworkPolicy); ok && np.Name == proxyEgressNetworkPolicyName { + return fmt.Errorf("proxy egress NP should not be created without proxy") + } + return nil + }) + }, + }, + { + name: "proxy Unmanaged, existing NP deleted", + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + if np, ok := obj.(*networkingv1.NetworkPolicy); ok { + existing := &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: proxyEgressNetworkPolicyName, + Namespace: externalsecretsDefaultNamespace, + }, + } + existing.DeepCopyInto(np) + } + return true, nil + }) + m.DeleteCalls(func(ctx context.Context, obj client.Object, opts ...client.DeleteOption) error { + return nil + }) + }, + updateExternalSecretsConfig: func(esc *operatorv1alpha1.ExternalSecretsConfig) { + esc.Spec.ApplicationConfig.Proxy = &operatorv1alpha1.ProxyConfig{ + HTTPSProxy: "https://proxy.example.com:8080", + NetworkPolicyAllowProxyEgressAll: operatorv1alpha1.Unmanaged, + } + }, + }, + { + name: "proxy Managed, NP created", + 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 { + if np, ok := obj.(*networkingv1.NetworkPolicy); ok { + if np.Name != proxyEgressNetworkPolicyName { + return fmt.Errorf("unexpected NP name: %s", np.Name) + } + } + return nil + }) + }, + updateExternalSecretsConfig: func(esc *operatorv1alpha1.ExternalSecretsConfig) { + esc.Spec.ApplicationConfig.Proxy = &operatorv1alpha1.ProxyConfig{ + HTTPSProxy: "https://proxy.example.com:8080", + NetworkPolicyAllowProxyEgressAll: operatorv1alpha1.Managed, + } + }, + }, + { + name: "proxy Managed, existing NP updated", + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + if np, ok := obj.(*networkingv1.NetworkPolicy); ok { + existing := &networkingv1.NetworkPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: proxyEgressNetworkPolicyName, + Namespace: externalsecretsDefaultNamespace, + }, + } + existing.DeepCopyInto(np) + } + return true, nil + }) + m.UpdateWithRetryCalls(func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { + return nil + }) + }, + updateExternalSecretsConfig: func(esc *operatorv1alpha1.ExternalSecretsConfig) { + esc.Spec.ApplicationConfig.Proxy = &operatorv1alpha1.ProxyConfig{ + HTTPSProxy: "https://proxy.example.com:8080", + NetworkPolicyAllowProxyEgressAll: operatorv1alpha1.Managed, + } + }, + }, + { + name: "proxy Managed, NP creation fails", + 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 { + if _, ok := obj.(*networkingv1.NetworkPolicy); ok { + return commontest.ErrTestClient + } + return nil + }) + }, + updateExternalSecretsConfig: func(esc *operatorv1alpha1.ExternalSecretsConfig) { + esc.Spec.ApplicationConfig.Proxy = &operatorv1alpha1.ProxyConfig{ + HTTPSProxy: "https://proxy.example.com:8080", + NetworkPolicyAllowProxyEgressAll: operatorv1alpha1.Managed, + } + }, + wantErr: "failed to create proxy egress network policy " + proxyEgressNPName + ": test client error", + }, + { + name: "proxy Unmanaged, Exists check fails", + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + return false, commontest.ErrTestClient + }) + }, + updateExternalSecretsConfig: func(esc *operatorv1alpha1.ExternalSecretsConfig) { + esc.Spec.ApplicationConfig.Proxy = &operatorv1alpha1.ProxyConfig{ + HTTPSProxy: "https://proxy.example.com:8080", + NetworkPolicyAllowProxyEgressAll: operatorv1alpha1.Unmanaged, + } + }, + wantErr: "failed to check existence of proxy egress network policy " + proxyEgressNPName + ": test client error", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r := testReconciler(t) + mock := &fakes.FakeCtrlClient{} + r.CtrlClient = mock + if tt.preReq != nil { + tt.preReq(r, mock) + } + + esc := commontest.TestExternalSecretsConfig() + if tt.updateExternalSecretsConfig != nil { + tt.updateExternalSecretsConfig(esc) + } + + err := r.createOrApplyProxyEgressNetworkPolicy(esc, testResourceMetadata(esc), false) + if tt.wantErr != "" { + if err == nil || err.Error() != tt.wantErr { + t.Errorf("Expected error: %v, got: %v", tt.wantErr, err) + } + } else if err != nil { + t.Errorf("Unexpected error: %v", err) + } + }) + } +} + +func TestCleanupMigratedNetworkPolicies(t *testing.T) { + tests := []struct { + name string + preReq func(*Reconciler, *fakes.FakeCtrlClient) + updateExternalSecretsConfig func(*operatorv1alpha1.ExternalSecretsConfig) + wantErr string + }{ + { + name: "no stale policies, migration already done", + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + m.ListCalls(func(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error { + // Return empty list — nothing to prune. + return nil + }) + m.DeleteCalls(func(ctx context.Context, obj client.Object, opts ...client.DeleteOption) error { + return fmt.Errorf("Delete should not be called when no stale policies exist") + }) + }, + updateExternalSecretsConfig: func(esc *operatorv1alpha1.ExternalSecretsConfig) { + esc.Annotations = map[string]string{ + migrationCompleteAnnotation: "true", + } + }, + }, + { + name: "stale labeled policy pruned", + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + m.ListCalls(func(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error { + if npList, ok := list.(*networkingv1.NetworkPolicyList); ok { + npList.Items = []networkingv1.NetworkPolicy{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "eso-user-old-policy", + Namespace: externalsecretsDefaultNamespace, + }, + }, + } + } + return nil + }) + m.DeleteCalls(func(ctx context.Context, obj client.Object, opts ...client.DeleteOption) error { + return nil + }) + }, + updateExternalSecretsConfig: func(esc *operatorv1alpha1.ExternalSecretsConfig) { + esc.Annotations = map[string]string{ + migrationCompleteAnnotation: "true", + } + // No NetworkPolicies in spec — so "eso-user-old-policy" is stale. + }, + }, + { + name: "first reconcile without annotation triggers legacy cleanup and sets annotation", + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + m.ListCalls(func(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error { + return nil // No labeled stale policies. + }) + m.ExistsCalls(func(ctx context.Context, ns types.NamespacedName, obj client.Object) (bool, error) { + // Simulate one legacy NP existing. + if ns.Name == "allow-api-server-egress" { + if np, ok := obj.(*networkingv1.NetworkPolicy); ok { + np.Name = ns.Name + np.Namespace = ns.Namespace + } + return true, nil + } + return false, nil + }) + m.DeleteCalls(func(ctx context.Context, obj client.Object, opts ...client.DeleteOption) error { + return nil + }) + m.PatchCalls(func(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error { + return nil + }) + }, + // No annotation on ESC — triggers migration path. + }, + { + name: "List fails returns error", + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + m.ListCalls(func(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error { + return commontest.ErrTestClient + }) + }, + wantErr: "failed to list network policies in namespace external-secrets: test client error", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r := testReconciler(t) + mock := &fakes.FakeCtrlClient{} + r.CtrlClient = mock + if tt.preReq != nil { + tt.preReq(r, mock) + } + + esc := commontest.TestExternalSecretsConfig() + if tt.updateExternalSecretsConfig != nil { + tt.updateExternalSecretsConfig(esc) + } + + err := r.cleanupMigratedNetworkPolicies(esc, testResourceMetadata(esc)) + if tt.wantErr != "" { + if err == nil || err.Error() != tt.wantErr { + t.Errorf("Expected error: %v, got: %v", tt.wantErr, err) + } + } else if err != nil { + t.Errorf("Unexpected error: %v", err) + } + }) + } +} diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index 1a462a855..4ff20c287 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -29,9 +29,11 @@ import ( "time" corev1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/client-go/dynamic" "k8s.io/client-go/kubernetes" "k8s.io/client-go/util/retry" @@ -713,6 +715,176 @@ var _ = Describe("External Secrets Operator End-to-End test scenarios", Ordered, }) }) + Context("Proxy Egress NetworkPolicy", func() { + const ( + proxyEgressNPName = "eso-sys-proxy-egress-core" + testProxyURL = "http://proxy.test.example.com:3128" + testProxyPort = 3128 + ) + + // restoreESCProxyConfig clears proxy and user-defined NetworkPolicy fields from the CR. + restoreESCProxyConfig := func() { + err := retry.RetryOnConflict(retry.DefaultRetry, func() error { + existingCR := &operatorv1alpha1.ExternalSecretsConfig{} + if err := runtimeClient.Get(ctx, client.ObjectKey{Name: "cluster"}, existingCR); err != nil { + return err + } + updatedCR := existingCR.DeepCopy() + updatedCR.Spec.ApplicationConfig.Proxy = nil + updatedCR.Spec.ControllerConfig.NetworkPolicies = nil + return runtimeClient.Update(ctx, updatedCR) + }) + Expect(err).NotTo(HaveOccurred(), "should restore ExternalSecretsConfig proxy config to baseline") + } + + It("should not create eso-sys-proxy-egress-core when no proxy is configured", func() { + By("Verifying eso-sys-proxy-egress-core does not exist without proxy configuration") + Consistently(func(g Gomega) { + _, err := clientset.NetworkingV1().NetworkPolicies(operandNamespace).Get(ctx, proxyEgressNPName, metav1.GetOptions{}) + g.Expect(err).To(HaveOccurred(), "%s should not exist when no proxy is configured", proxyEgressNPName) + }, 15*time.Second, 3*time.Second).Should(Succeed()) + }) + + It("should create eso-sys-proxy-egress-core when proxy is Managed and proxy URL is set", func() { + defer restoreESCProxyConfig() + + By("Setting proxy with Managed mode in ExternalSecretsConfig") + err := retry.RetryOnConflict(retry.DefaultRetry, func() error { + existingCR := &operatorv1alpha1.ExternalSecretsConfig{} + if err := runtimeClient.Get(ctx, client.ObjectKey{Name: "cluster"}, existingCR); err != nil { + return err + } + updatedCR := existingCR.DeepCopy() + updatedCR.Spec.ApplicationConfig.Proxy = &operatorv1alpha1.ProxyConfig{ + HTTPProxy: testProxyURL, + NetworkPolicyAllowProxyEgressAll: operatorv1alpha1.Managed, + } + return runtimeClient.Update(ctx, updatedCR) + }) + Expect(err).NotTo(HaveOccurred(), "should set proxy config in ExternalSecretsConfig") + + By("Verifying eso-sys-proxy-egress-core NetworkPolicy is created with correct egress port") + Eventually(func(g Gomega) { + np, err := clientset.NetworkingV1().NetworkPolicies(operandNamespace).Get(ctx, proxyEgressNPName, metav1.GetOptions{}) + g.Expect(err).NotTo(HaveOccurred(), "%s should be created when proxy Managed", proxyEgressNPName) + g.Expect(np.Spec.Egress).NotTo(BeEmpty(), "NP should have egress rules") + + var foundPort bool + for _, rule := range np.Spec.Egress { + for _, port := range rule.Ports { + if port.Port != nil && port.Port.IntValue() == testProxyPort { + foundPort = true + } + } + } + g.Expect(foundPort).To(BeTrue(), "NP should allow egress to proxy port %d", testProxyPort) + }, 2*time.Minute, 5*time.Second).Should(Succeed()) + }) + + It("should delete eso-sys-proxy-egress-core when mode switches to Unmanaged", func() { + defer restoreESCProxyConfig() + + By("Setting proxy with Managed mode to create the NP") + err := retry.RetryOnConflict(retry.DefaultRetry, func() error { + existingCR := &operatorv1alpha1.ExternalSecretsConfig{} + if err := runtimeClient.Get(ctx, client.ObjectKey{Name: "cluster"}, existingCR); err != nil { + return err + } + updatedCR := existingCR.DeepCopy() + updatedCR.Spec.ApplicationConfig.Proxy = &operatorv1alpha1.ProxyConfig{ + HTTPProxy: testProxyURL, + NetworkPolicyAllowProxyEgressAll: operatorv1alpha1.Managed, + } + return runtimeClient.Update(ctx, updatedCR) + }) + Expect(err).NotTo(HaveOccurred()) + + By("Waiting for eso-sys-proxy-egress-core to be created") + Eventually(func(g Gomega) { + _, err := clientset.NetworkingV1().NetworkPolicies(operandNamespace).Get(ctx, proxyEgressNPName, metav1.GetOptions{}) + g.Expect(err).NotTo(HaveOccurred()) + }, 2*time.Minute, 5*time.Second).Should(Succeed()) + + By("Switching networkPolicyAllowProxyEgressAll to Unmanaged") + err = retry.RetryOnConflict(retry.DefaultRetry, func() error { + existingCR := &operatorv1alpha1.ExternalSecretsConfig{} + if err := runtimeClient.Get(ctx, client.ObjectKey{Name: "cluster"}, existingCR); err != nil { + return err + } + updatedCR := existingCR.DeepCopy() + updatedCR.Spec.ApplicationConfig.Proxy = &operatorv1alpha1.ProxyConfig{ + HTTPProxy: testProxyURL, + NetworkPolicyAllowProxyEgressAll: operatorv1alpha1.Unmanaged, + } + return runtimeClient.Update(ctx, updatedCR) + }) + Expect(err).NotTo(HaveOccurred()) + + By("Verifying eso-sys-proxy-egress-core is deleted after switching to Unmanaged") + Eventually(func(g Gomega) { + _, err := clientset.NetworkingV1().NetworkPolicies(operandNamespace).Get(ctx, proxyEgressNPName, metav1.GetOptions{}) + g.Expect(err).To(HaveOccurred(), "%s should be deleted when mode is Unmanaged", proxyEgressNPName) + }, 2*time.Minute, 5*time.Second).Should(Succeed()) + }) + + It("should apply eso-user- prefix to user-defined NetworkPolicy names", func() { + defer restoreESCProxyConfig() + + const userPolicyName = "my-custom-egress" + const prefixedPolicyName = "eso-user-" + userPolicyName + + By("Adding a user-defined NetworkPolicy to ExternalSecretsConfig spec") + tcpProto := corev1.ProtocolTCP + port443 := intstr.FromInt32(443) + err := retry.RetryOnConflict(retry.DefaultRetry, func() error { + existingCR := &operatorv1alpha1.ExternalSecretsConfig{} + if err := runtimeClient.Get(ctx, client.ObjectKey{Name: "cluster"}, existingCR); err != nil { + return err + } + updatedCR := existingCR.DeepCopy() + updatedCR.Spec.ControllerConfig.NetworkPolicies = []operatorv1alpha1.NetworkPolicy{ + { + Name: userPolicyName, + ComponentName: operatorv1alpha1.CoreController, + Egress: []networkingv1.NetworkPolicyEgressRule{ + { + Ports: []networkingv1.NetworkPolicyPort{ + {Protocol: &tcpProto, Port: &port443}, + }, + }, + }, + }, + } + return runtimeClient.Update(ctx, updatedCR) + }) + Expect(err).NotTo(HaveOccurred(), "should add user-defined NetworkPolicy to ExternalSecretsConfig") + + By(fmt.Sprintf("Verifying %s exists (with eso-user- prefix)", prefixedPolicyName)) + Eventually(func(g Gomega) { + np, err := clientset.NetworkingV1().NetworkPolicies(operandNamespace).Get(ctx, prefixedPolicyName, metav1.GetOptions{}) + g.Expect(err).NotTo(HaveOccurred(), "%s should exist", prefixedPolicyName) + g.Expect(np.Spec.Egress).NotTo(BeEmpty()) + }, 2*time.Minute, 5*time.Second).Should(Succeed()) + + By(fmt.Sprintf("Verifying unprefixed name '%s' does not exist as a separate object", userPolicyName)) + _, err = clientset.NetworkingV1().NetworkPolicies(operandNamespace).Get(ctx, userPolicyName, metav1.GetOptions{}) + Expect(err).To(HaveOccurred(), "unprefixed '%s' should not exist as a K8s object", userPolicyName) + }) + + It("should set migration complete annotation on ExternalSecretsConfig after first reconcile", func() { + By("Verifying migration complete annotation is set on ExternalSecretsConfig") + Eventually(func(g Gomega) { + existingCR := &operatorv1alpha1.ExternalSecretsConfig{} + err := runtimeClient.Get(ctx, client.ObjectKey{Name: "cluster"}, existingCR) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(existingCR.GetAnnotations()).To( + HaveKey("operator.openshift.io/network-policy-migration-complete"), + "migration complete annotation should be set after first reconcile", + ) + }, 2*time.Minute, 5*time.Second).Should(Succeed()) + }) + }) + AfterAll(func() { By("Deleting the externalsecrets.openshift.operator.io/cluster CR") loader.DeleteFromFile(testassets.ReadFile, externalSecretsFile, "")