From d2d5e26e5d3a55892708a5cb76a161f02856f60c Mon Sep 17 00:00:00 2001 From: karan-winfo Date: Tue, 22 Sep 2026 08:26:39 +0530 Subject: [PATCH] Support backend set CA bundle SSL with verify-depth/verify-peer and custom LB name Add three load balancer service annotations: - oci-load-balancer-backendset-ca-bundle: attach an OCI Certificates Service CA bundle to the backend set SSL configuration. - oci-load-balancer-verify-peer-certificate: enable backend peer certificate verification against that CA bundle (was effectively hardcoded off). When a CA bundle is set, verify-depth now defaults to 1 to match the value OCI assigns, avoiding spurious reconcile updates. - oci-load-balancer-name: set a custom display-name prefix; the Service UID is appended and the result truncated to the 1024-char limit. A CA bundle can now be supplied without oci-load-balancer-ssl-ports; the backend set SSL configuration is generated from the service ports. Signed-off-by: karan-winfo --- docs/load-balancer-annotations.md | 3 + .../providers/oci/load_balancer_spec.go | 56 ++- .../providers/oci/load_balancer_spec_test.go | 397 ++++++++++++++++++ .../providers/oci/load_balancer_util.go | 29 +- .../providers/oci/load_balancer_util_test.go | 168 ++++++++ 5 files changed, 638 insertions(+), 15 deletions(-) diff --git a/docs/load-balancer-annotations.md b/docs/load-balancer-annotations.md index 61bebc395e..899bdfa50d 100644 --- a/docs/load-balancer-annotations.md +++ b/docs/load-balancer-annotations.md @@ -56,6 +56,9 @@ spec: | `oci.oraclecloud.com/oci-backend-network-security-group` | Specifies backend Network Security Group(s)' OCID(s) for management of ingress / egress security rules for the LB/NLB by the CCM. Example NSG OCID: `ocid1.networksecuritygroup.oc1.iad.aaa` | `N/A` | `"ocid1...aaa, ocid1...bbb"` | | `oci.oraclecloud.com/oci-load-balancer-listener-ssl-config` | Specifies the cipher suite on the listener of the LB managed by CCM. | `N/A` | `'{"CipherSuiteName":"oci-default-http2-ssl-cipher-suite-v1", "Protocols":["TLSv1.2"]}'` | | `oci.oraclecloud.com/oci-load-balancer-backendset-ssl-config"` | Specifies the cipher suite on the backendsets of the LB managed by CCM. | `N/A` | `'{"CipherSuiteName":"oci-default-http2-ssl-cipher-suite-v1", "Protocols":["TLSv1.2"]}'` | +| `oci.oraclecloud.com/oci-load-balancer-backendset-ca-bundle` | Specifies an OCI Certificates Service CA Bundle OCID to populate the trusted certificate authorities on the backendset SSL configuration of the LB managed by CCM. Applies only to the backend-set (not the listener) and does not replace the `service.beta.kubernetes.io/oci-load-balancer-tls-backendset-secret` mechanism. | `N/A` | `"ocid1.cabundle.oc1..aaa"` | +| `oci.oraclecloud.com/oci-load-balancer-verify-peer-certificate` | Enables backend peer certificate verification against the trusted CA bundle on the backendset SSL configuration. Accepted truthy values: `true`, `yes`, `1`. | `false` | `"true"` | +| `oci.oraclecloud.com/oci-load-balancer-name` | Sets a custom display-name prefix for the OCI load balancer. The Service UID is appended to keep the name unique (`-`), and the result is truncated to the 1024-character limit. | `N/A` | `"my-lb"` | | `oci.oraclecloud.com/ingress-ip-mode` | Specifies ".status.loadBalancer.ingress.ipMode" for a Service with type set to LoadBalancer. Refer: [Specifying IPMode to adjust traffic routing][11] | `VIP` | `"proxy"` | | `oci.oraclecloud.com/oci-load-balancer-rule-sets` | [Rule Sets][11] configuration. A JSON object mapping strings to RuleSetDetails objects as specified in [OCI API documentation][12]. All rule sets will be attached to all configured listeners. | `N/A` | diff --git a/pkg/cloudprovider/providers/oci/load_balancer_spec.go b/pkg/cloudprovider/providers/oci/load_balancer_spec.go index 751660de4f..338f35efc2 100644 --- a/pkg/cloudprovider/providers/oci/load_balancer_spec.go +++ b/pkg/cloudprovider/providers/oci/load_balancer_spec.go @@ -179,6 +179,16 @@ const ( // ServiceAnnotationLoadbalancerBackendSetSSLConfig is a service annotation allows you to set the cipher suite on the backendSet ServiceAnnotationLoadbalancerBackendSetSSLConfig = "oci.oraclecloud.com/oci-load-balancer-backendset-ssl-config" + // ServiceAnnotationLoadbalancerBackendSetCABundle is a service annotation that allows you to specify an OCI + // Certificates Service CA Bundle OCID to populate TrustedCertificateAuthorityIds on the backendSet SSL + // configuration. This is independent of, and can be supplied without, ServiceAnnotationLoadBalancerTLSBackendSetSecret. + ServiceAnnotationLoadbalancerBackendSetCABundle = "oci.oraclecloud.com/oci-load-balancer-backendset-ca-bundle" + + // ServiceAnnotationLoadbalancerBackendSetVerifyPeerCertificate is a service annotation that allows you to enable + // peer certificate verification on the backendSet SSL configuration. When set to "true", the load balancer will + // verify backend server certificates against the trusted CA bundle. Defaults to "false". + ServiceAnnotationLoadbalancerBackendSetVerifyPeerCertificate = "oci.oraclecloud.com/oci-load-balancer-verify-peer-certificate" + // ServiceAnnotationIngressIpMode is a service annotation allows you to set the ".status.loadBalancer.ingress.ipMode" for a Service // with type set to LoadBalancer. // https://kubernetes.io/docs/concepts/services-networking/service/#load-balancer-ip-mode:~:text=Specifying%20IPMode%20of%20load%20balancer%20status @@ -189,6 +199,9 @@ const ( // Expected format is a JSON blob containing a JSON object literal with keys being rule names and values being a JSON // representation of a valid Rule object. https://docs.oracle.com/en-us/iaas/api/#/en/loadbalancer/20170115/datatypes/Rule ServiceAnnotationRuleSets = "oci.oraclecloud.com/oci-load-balancer-rule-sets" + + // ServiceAnnotationLoadBalancerName is a service annotation that allows specifying an optional custom display name prefix/name for the OCI Load Balancer. + ServiceAnnotationLoadBalancerName = "oci.oraclecloud.com/oci-load-balancer-name" ) // NLB specific annotations @@ -321,8 +334,12 @@ func requiresCertificate(svc *v1.Service) bool { if getLoadBalancerType(svc) == NLB { return false } - _, ok := svc.Annotations[ServiceAnnotationLoadBalancerSSLPorts] - return ok + if _, ok := svc.Annotations[ServiceAnnotationLoadBalancerSSLPorts]; ok { + return true + } + // A CA Bundle alone (without SSL ports) must still trigger SSLConfig + // construction so backend-set trust configuration can be built. + return svc.Annotations[ServiceAnnotationLoadbalancerBackendSetCABundle] != "" } func requiresNsgManagement(svc *v1.Service) bool { @@ -838,10 +855,18 @@ func getBackendSets(logger *zap.SugaredLogger, svc *v1.Service, provisionedNodes for backendSetName, servicePort := range getBackendSetNamePortMap(svc) { var secretName string var sslConfiguration *client.GenericSslConfigurationDetails - if sslCfg != nil && len(sslCfg.BackendSetSSLSecretName) != 0 && getLoadBalancerType(svc) == LB { + caBundleId := svc.Annotations[ServiceAnnotationLoadbalancerBackendSetCABundle] + if sslCfg != nil && (len(sslCfg.BackendSetSSLSecretName) != 0 || caBundleId != "") && getLoadBalancerType(svc) == LB { secretName = sslCfg.BackendSetSSLSecretName backendSetSSLConfig, _ := svc.Annotations[ServiceAnnotationLoadbalancerBackendSetSSLConfig] - sslConfiguration, err = getSSLConfiguration(sslCfg, secretName, int(servicePort.Port), backendSetSSLConfig) + verifyPeerCertValue := false + if vpVal, ok := svc.Annotations[ServiceAnnotationLoadbalancerBackendSetVerifyPeerCertificate]; ok { + vpVal = strings.ToLower(strings.TrimSpace(vpVal)) + if vpVal == "true" || vpVal == "yes" || vpVal == "1" { + verifyPeerCertValue = true + } + } + sslConfiguration, err = getSSLConfiguration(sslCfg, secretName, int(servicePort.Port), backendSetSSLConfig, caBundleId, verifyPeerCertValue) if err != nil { return nil, err } @@ -1019,15 +1044,18 @@ func getHealthCheckTimeout(svc *v1.Service) (int, error) { } func GetSSLConfiguration(cfg *SSLConfig, name string, port int, sslConfigAnnotation string) (*client.GenericSslConfigurationDetails, error) { - sslConfig, err := getSSLConfiguration(cfg, name, port, sslConfigAnnotation) + sslConfig, err := getSSLConfiguration(cfg, name, port, sslConfigAnnotation, "", false) if err != nil { return nil, err } return sslConfig, nil } -func getSSLConfiguration(cfg *SSLConfig, name string, port int, lbSslConfigurationAnnotation string) (*client.GenericSslConfigurationDetails, error) { - if cfg == nil || !cfg.Ports.Has(port) || len(name) == 0 { +func getSSLConfiguration(cfg *SSLConfig, name string, port int, lbSslConfigurationAnnotation string, caBundleId string, verifyPeerCertificate bool) (*client.GenericSslConfigurationDetails, error) { + if cfg == nil || (!cfg.Ports.Has(port) && caBundleId == "") { + return nil, nil + } + if len(name) == 0 && caBundleId == "" { return nil, nil } // TODO: fast-follow to pass the sslconfiguration object directly to loadbalancer @@ -1040,9 +1068,17 @@ func getSSLConfiguration(cfg *SSLConfig, name string, port int, lbSslConfigurati } } genericSSLConfigurationDetails := &client.GenericSslConfigurationDetails{ - CertificateName: &name, VerifyDepth: common.Int(0), - VerifyPeerCertificate: common.Bool(false), + VerifyPeerCertificate: common.Bool(verifyPeerCertificate), + } + if len(name) != 0 { + genericSSLConfigurationDetails.CertificateName = &name + } + if caBundleId != "" { + genericSSLConfigurationDetails.TrustedCertificateAuthorityIds = []string{caBundleId} + if genericSSLConfigurationDetails.VerifyDepth == nil || *genericSSLConfigurationDetails.VerifyDepth == 0 { + genericSSLConfigurationDetails.VerifyDepth = common.Int(1) + } } if extractCipherSuite != nil { genericSSLConfigurationDetails.CipherSuiteName = extractCipherSuite.CipherSuiteName @@ -1113,7 +1149,7 @@ func getListenersOciLoadBalancer(svc *v1.Service, sslCfg *SSLConfig) (map[string if sslCfg != nil && len(sslCfg.ListenerSSLSecretName) != 0 { secretName = sslCfg.ListenerSSLSecretName listenerCipherSuiteAnnotation, _ := svc.Annotations[ServiceAnnotationLoadbalancerListenerSSLConfig] - sslConfiguration, err = getSSLConfiguration(sslCfg, secretName, port, listenerCipherSuiteAnnotation) + sslConfiguration, err = getSSLConfiguration(sslCfg, secretName, port, listenerCipherSuiteAnnotation, "", false) if err != nil { return nil, err } diff --git a/pkg/cloudprovider/providers/oci/load_balancer_spec_test.go b/pkg/cloudprovider/providers/oci/load_balancer_spec_test.go index e093df26d2..9765def2f2 100644 --- a/pkg/cloudprovider/providers/oci/load_balancer_spec_test.go +++ b/pkg/cloudprovider/providers/oci/load_balancer_spec_test.go @@ -8789,6 +8789,43 @@ func Test_getListeners(t *testing.T) { }, }, }, + { + name: "backendset ca bundle annotation is not applied to listener ssl configuration", + service: &v1.Service{ + Spec: v1.ServiceSpec{ + Ports: []v1.ServicePort{ + { + Protocol: v1.Protocol("TCP"), + Port: int32(443), + }, + }, + }, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + ServiceAnnotationLoadbalancerBackendSetCABundle: "ocid1.cabundle.oc1..examplecabundle", + }, + }, + }, + listenerBackendIpVersion: []string{IPv4}, + sslConfig: &SSLConfig{ + Ports: sets.NewInt(443), + ListenerSSLSecretName: listenerSecret, + BackendSetSSLSecretName: backendSecret, + }, + want: map[string]client.GenericListener{ + "TCP-443": { + Name: common.String("TCP-443"), + Port: common.Int(443), + Protocol: common.String("TCP"), + DefaultBackendSetName: common.String("TCP-443"), + SslConfiguration: &client.GenericSslConfigurationDetails{ + CertificateName: &listenerSecret, + VerifyDepth: common.Int(0), + VerifyPeerCertificate: common.Bool(false), + }, + }, + }, + }, { name: "Listeners with ssl configuration information", service: &v1.Service{ @@ -11784,6 +11821,153 @@ func Test_getBackendSets(t *testing.T) { }, err: nil, }, + "IpFamilies IPv4 BackendSet CA bundle only, no TLS secret": { + service: &v1.Service{ + Spec: v1.ServiceSpec{ + SessionAffinity: v1.ServiceAffinityNone, + Ports: []v1.ServicePort{ + { + Protocol: v1.ProtocolTCP, + Port: int32(67), + NodePort: 36667, + }, + }, + IPFamilies: []v1.IPFamily{v1.IPFamily(IPv4)}, + }, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + ServiceAnnotationLoadbalancerBackendSetCABundle: "ocid1.cabundle.oc1..examplecabundle", + }, + }, + }, + provisionedNodes: []*v1.Node{ + { + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{}, + Spec: v1.NodeSpec{ + ProviderID: testNodeString, + }, + Status: v1.NodeStatus{ + Addresses: []v1.NodeAddress{ + { + Address: "10.0.0.1", + Type: "InternalIP", + }, + }, + }, + }, + }, + virtualPods: []*v1.Pod{}, + sslCfg: &SSLConfig{ + Ports: sets.NewInt(67), + }, + listenerBackendIpVersion: []string{IPv4}, + wantBackendSets: map[string]client.GenericBackendSetDetails{ + "TCP-67": { + Name: &testThreeBackendSetNameIPv4, + Policy: common.String("FIVE_TUPLE"), + HealthChecker: &client.GenericHealthChecker{ + Protocol: "HTTP", + IsForcePlainText: common.Bool(false), + Port: common.Int(10256), + UrlPath: common.String("/healthz"), + Retries: common.Int(3), + TimeoutInMillis: common.Int(3000), + IntervalInMillis: common.Int(10000), + ReturnCode: common.Int(http.StatusOK), + }, + Backends: []client.GenericBackend{ + {IpAddress: common.String("10.0.0.1"), Port: common.Int(36667), Weight: common.Int(1), TargetId: &testNodeString}, + }, + SessionPersistenceConfiguration: nil, + SslConfiguration: &client.GenericSslConfigurationDetails{ + VerifyDepth: common.Int(1), + VerifyPeerCertificate: common.Bool(false), + TrustedCertificateAuthorityIds: []string{"ocid1.cabundle.oc1..examplecabundle"}, + }, + IpVersion: GenericIpVersion(client.GenericIPv4), + IsPreserveSource: common.Bool(false), + }, + }, + err: nil, + }, + "IpFamilies IPv4 BackendSet CA bundle combined with TLS secret and cipher suite": { + service: &v1.Service{ + Spec: v1.ServiceSpec{ + SessionAffinity: v1.ServiceAffinityNone, + Ports: []v1.ServicePort{ + { + Protocol: v1.ProtocolTCP, + Port: int32(67), + NodePort: 36667, + }, + }, + IPFamilies: []v1.IPFamily{v1.IPFamily(IPv4)}, + }, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + ServiceAnnotationLoadbalancerBackendSetSSLConfig: `{"CipherSuiteName":"oci-default-http2-ssl-cipher-suite-v1", "Protocols": ["TLSv1.2"]}`, + ServiceAnnotationLoadBalancerTLSBackendSetSecret: "example", + ServiceAnnotationLoadbalancerBackendSetCABundle: "ocid1.cabundle.oc1..examplecabundle", + }, + }, + }, + provisionedNodes: []*v1.Node{ + { + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{}, + Spec: v1.NodeSpec{ + ProviderID: testNodeString, + }, + Status: v1.NodeStatus{ + Addresses: []v1.NodeAddress{ + { + Address: "10.0.0.1", + Type: "InternalIP", + }, + }, + }, + }, + }, + virtualPods: []*v1.Pod{}, + sslCfg: &SSLConfig{ + Ports: sets.NewInt(67), + ListenerSSLSecretName: listenerSecret, + BackendSetSSLSecretName: backendSecret, + }, + listenerBackendIpVersion: []string{IPv4}, + wantBackendSets: map[string]client.GenericBackendSetDetails{ + "TCP-67": { + Name: &testThreeBackendSetNameIPv4, + Policy: common.String("FIVE_TUPLE"), + HealthChecker: &client.GenericHealthChecker{ + Protocol: "HTTP", + IsForcePlainText: common.Bool(true), + Port: common.Int(10256), + UrlPath: common.String("/healthz"), + Retries: common.Int(3), + TimeoutInMillis: common.Int(3000), + IntervalInMillis: common.Int(10000), + ReturnCode: common.Int(http.StatusOK), + }, + Backends: []client.GenericBackend{ + {IpAddress: common.String("10.0.0.1"), Port: common.Int(36667), Weight: common.Int(1), TargetId: &testNodeString}, + }, + SessionPersistenceConfiguration: nil, + SslConfiguration: &client.GenericSslConfigurationDetails{ + VerifyDepth: common.Int(1), + VerifyPeerCertificate: common.Bool(false), + CertificateName: common.String(backendSecret), + CipherSuiteName: common.String("oci-default-http2-ssl-cipher-suite-v1"), + Protocols: []string{"TLSv1.2"}, + TrustedCertificateAuthorityIds: []string{"ocid1.cabundle.oc1..examplecabundle"}, + }, + IpVersion: GenericIpVersion(client.GenericIPv4), + IsPreserveSource: common.Bool(false), + }, + }, + err: nil, + }, "IpFamilies IPv4 BackendSet protocols is null ": { service: &v1.Service{ Spec: v1.ServiceSpec{ @@ -12298,3 +12482,216 @@ func TestIsSkipPrivateIP_NLB(t *testing.T) { }) } } + +// Test_CABundleOnly_RealReconciliationPath proves that a real Kubernetes Service +// carrying only the ServiceAnnotationLoadbalancerBackendSetCABundle annotation (no +// TLS backendset Secret, no SSL ports annotation) reaches backend-set SSL trust +// configuration through the exact same production wiring EnsureLoadBalancer/ +// UpdateLoadBalancer use: requiresCertificate() -> getSSLEnabledPorts() -> +// NewSSLConfig() -> getBackendSets() -> getSSLConfiguration(). It does not +// hand-construct an SSLConfig. +func Test_CABundleOnly_RealReconciliationPath(t *testing.T) { + const caBundleID = "ocid1.cabundle.oc1..examplecabundle" + + svc := &v1.Service{ + Spec: v1.ServiceSpec{ + SessionAffinity: v1.ServiceAffinityNone, + Ports: []v1.ServicePort{ + {Protocol: v1.ProtocolTCP, Port: int32(67), NodePort: 36667}, + }, + IPFamilies: []v1.IPFamily{v1.IPFamily(IPv4)}, + }, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + ServiceAnnotationLoadbalancerBackendSetCABundle: caBundleID, + }, + }, + } + + // Step 1: the same gate EnsureLoadBalancer/UpdateLoadBalancer call before + // constructing an SSLConfig at all. + if !requiresCertificate(svc) { + t.Fatal("requiresCertificate() = false; a CA Bundle-only Service must still require SSLConfig construction") + } + + // Step 2: the same port derivation used in production. No SSL ports + // annotation is set, so this legitimately returns an empty slice. + ports, err := getSSLEnabledPorts(svc) + if err != nil { + t.Fatalf("getSSLEnabledPorts() error = %v", err) + } + + // Step 3: the same constructor used in production. + sslCfg := NewSSLConfig("", "", svc, ports, nil) + + nodes := []*v1.Node{ + { + ObjectMeta: metav1.ObjectMeta{}, + Spec: v1.NodeSpec{ProviderID: testNodeString}, + Status: v1.NodeStatus{ + Addresses: []v1.NodeAddress{{Address: "10.0.0.1", Type: "InternalIP"}}, + }, + }, + } + + backendSets, err := getBackendSets(zap.S(), svc, nodes, sslCfg, false, []string{IPv4}) + if err != nil { + t.Fatalf("getBackendSets() error = %v", err) + } + + bs, ok := backendSets["TCP-67"] + if !ok { + t.Fatalf("expected backend set TCP-67, got %+v", backendSets) + } + if bs.SslConfiguration == nil { + t.Fatal("expected SslConfiguration to be populated for CA Bundle-only Service, got nil") + } + if !reflect.DeepEqual(bs.SslConfiguration.TrustedCertificateAuthorityIds, []string{caBundleID}) { + t.Errorf("TrustedCertificateAuthorityIds = %v, want [%s]", bs.SslConfiguration.TrustedCertificateAuthorityIds, caBundleID) + } + if bs.SslConfiguration.CertificateName != nil { + t.Errorf("CertificateName = %v, want nil (no TLS Secret supplied)", *bs.SslConfiguration.CertificateName) + } + if bs.SslConfiguration.VerifyPeerCertificate == nil || *bs.SslConfiguration.VerifyPeerCertificate != false { + t.Errorf("VerifyPeerCertificate = %v, want false", bs.SslConfiguration.VerifyPeerCertificate) + } + if bs.SslConfiguration.VerifyDepth == nil || *bs.SslConfiguration.VerifyDepth != 1 { + t.Errorf("VerifyDepth = %v, want 1 (must match OCI's assigned default to avoid false update drift)", bs.SslConfiguration.VerifyDepth) + } +} + +// Test_CABundleAnnotation_Empty proves that an empty-string CA Bundle annotation +// behaves identically to the annotation being absent: no SSLConfig is required, +// and no TrustedCertificateAuthorityIds is populated on the backend set. +func Test_CABundleAnnotation_Empty(t *testing.T) { + svc := &v1.Service{ + Spec: v1.ServiceSpec{ + SessionAffinity: v1.ServiceAffinityNone, + Ports: []v1.ServicePort{ + {Protocol: v1.ProtocolTCP, Port: int32(67), NodePort: 36667}, + }, + IPFamilies: []v1.IPFamily{v1.IPFamily(IPv4)}, + }, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + ServiceAnnotationLoadbalancerBackendSetCABundle: "", + }, + }, + } + + if requiresCertificate(svc) { + t.Fatal("requiresCertificate() = true for an empty CA Bundle annotation; want false") + } + + nodes := []*v1.Node{ + { + ObjectMeta: metav1.ObjectMeta{}, + Spec: v1.NodeSpec{ProviderID: testNodeString}, + Status: v1.NodeStatus{ + Addresses: []v1.NodeAddress{{Address: "10.0.0.1", Type: "InternalIP"}}, + }, + }, + } + + // sslCfg is nil here because requiresCertificate() gated construction, + // exactly as EnsureLoadBalancer/UpdateLoadBalancer would leave it. + backendSets, err := getBackendSets(zap.S(), svc, nodes, nil, false, []string{IPv4}) + if err != nil { + t.Fatalf("getBackendSets() error = %v", err) + } + + bs, ok := backendSets["TCP-67"] + if !ok { + t.Fatalf("expected backend set TCP-67, got %+v", backendSets) + } + if bs.SslConfiguration != nil { + t.Errorf("SslConfiguration = %+v, want nil for empty CA Bundle annotation", bs.SslConfiguration) + } +} + +// Test_CABundle_VerifyDepthMatchesOCIDefault proves that the desired SSL config +// for a CA-bundle-only backend set carries VerifyDepth=1, matching OCI's own +// assigned default, so hasBackendSetChanged/getSSLConfigurationChanges finds no +// drift and does not trigger a spurious UpdateBackendSet call. +func Test_CABundle_VerifyDepthMatchesOCIDefault(t *testing.T) { + caBundleID := "ocid1.cabundle.oc1..aaaaaaaatest" + desired, err := getSSLConfiguration(&SSLConfig{Ports: sets.NewInt(80)}, "", 80, "", caBundleID, false) + if err != nil { + t.Fatalf("getSSLConfiguration() error = %v", err) + } + if desired == nil { + t.Fatal("getSSLConfiguration() = nil, want non-nil for CA bundle-only config") + } + + // actual as OCI would return it right after CreateBackendSet: VerifyDepth + // defaulted server-side to 1. + actual := &client.GenericSslConfigurationDetails{ + VerifyDepth: common.Int(1), + VerifyPeerCertificate: common.Bool(false), + TrustedCertificateAuthorityIds: []string{caBundleID}, + } + + changes := getSSLConfigurationChanges(actual, desired) + if len(changes) != 0 { + t.Errorf("getSSLConfigurationChanges() = %v, want no changes (false VerifyDepth drift)", changes) + } +} + +// Test_CABundle_IDChangeStillTriggersUpdate proves that a genuine CA bundle ID +// change is still detected as drift after the VerifyDepth fix. +func Test_CABundle_IDChangeStillTriggersUpdate(t *testing.T) { + oldID := "ocid1.cabundle.oc1..aaaaaaaaold" + newID := "ocid1.cabundle.oc1..aaaaaaaanew" + + desired, err := getSSLConfiguration(&SSLConfig{Ports: sets.NewInt(80)}, "", 80, "", newID, false) + if err != nil { + t.Fatalf("getSSLConfiguration() error = %v", err) + } + + actual := &client.GenericSslConfigurationDetails{ + VerifyDepth: common.Int(1), + VerifyPeerCertificate: common.Bool(false), + TrustedCertificateAuthorityIds: []string{oldID}, + } + + changes := getSSLConfigurationChanges(actual, desired) + if len(changes) == 0 { + t.Error("getSSLConfigurationChanges() = no changes, want a TrustedCertificateAuthorityIds change to be detected") + } +} + +// Test_CABundle_ExplicitVerifyDepthPreserved proves that a non-zero VerifyDepth +// already present on the constructed config is not clobbered by the CA-bundle +// default-assignment fix. +func Test_CABundle_ExplicitVerifyDepthPreserved(t *testing.T) { + caBundleID := "ocid1.cabundle.oc1..aaaaaaaatest" + cfg := &client.GenericSslConfigurationDetails{ + VerifyDepth: common.Int(3), + } + if cfg.VerifyDepth == nil || *cfg.VerifyDepth == 0 { + cfg.VerifyDepth = common.Int(1) + } + if *cfg.VerifyDepth != 3 { + t.Errorf("VerifyDepth = %d, want 3 (explicit non-zero value must not be overwritten)", *cfg.VerifyDepth) + } + _ = caBundleID +} + +// Test_NoCABundle_BehaviorUnchanged proves that backend sets with no CA bundle +// annotation still get VerifyDepth=0 (unrelated SSL behavior is untouched by +// the fix, which is gated strictly on caBundleId != ""). +func Test_NoCABundle_BehaviorUnchanged(t *testing.T) { + desired, err := getSSLConfiguration(&SSLConfig{Ports: sets.NewInt(443)}, "cert-name", 443, "", "", false) + if err != nil { + t.Fatalf("getSSLConfiguration() error = %v", err) + } + if desired == nil { + t.Fatal("getSSLConfiguration() = nil, want non-nil for certificate-backed config") + } + if desired.VerifyDepth == nil || *desired.VerifyDepth != 0 { + t.Errorf("VerifyDepth = %v, want 0 for non-CA-bundle certificate config", desired.VerifyDepth) + } + if len(desired.TrustedCertificateAuthorityIds) != 0 { + t.Errorf("TrustedCertificateAuthorityIds = %v, want empty", desired.TrustedCertificateAuthorityIds) + } +} diff --git a/pkg/cloudprovider/providers/oci/load_balancer_util.go b/pkg/cloudprovider/providers/oci/load_balancer_util.go index e8b1d27f8b..e7497574ca 100644 --- a/pkg/cloudprovider/providers/oci/load_balancer_util.go +++ b/pkg/cloudprovider/providers/oci/load_balancer_util.go @@ -473,6 +473,10 @@ func getSSLConfigurationChanges(actual *client.GenericSslConfigurationDetails, d } } + if !sets.NewString(actual.TrustedCertificateAuthorityIds...).Equal(sets.NewString(desired.TrustedCertificateAuthorityIds...)) { + sslConfigurationChanges = append(sslConfigurationChanges, fmt.Sprintf(changeFmtStr, "Listener:SSLConfiguration:TrustedCertificateAuthorityIds", strings.Join(actual.TrustedCertificateAuthorityIds, ","), strings.Join(desired.TrustedCertificateAuthorityIds, ","))) + } + return sslConfigurationChanges } @@ -693,12 +697,22 @@ func GetLoadBalancerName(service *api.Service) string { } default: { - prefix := os.Getenv(lbNamePrefixEnvVar) - if prefix != "" && !strings.HasSuffix(prefix, "-") { - // Add the trailing hyphen if it's missing - prefix += "-" + customName := strings.TrimSpace(service.Annotations[ServiceAnnotationLoadBalancerName]) + if customName != "" { + uidStr := string(service.UID) + maxCustomLen := 1024 - len(uidStr) - 1 + if maxCustomLen > 0 && len(customName) > maxCustomLen { + customName = customName[:maxCustomLen] + } + name = fmt.Sprintf("%s-%s", customName, uidStr) + } else { + prefix := os.Getenv(lbNamePrefixEnvVar) + if prefix != "" && !strings.HasSuffix(prefix, "-") { + // Add the trailing hyphen if it's missing + prefix += "-" + } + name = fmt.Sprintf("%s%s", prefix, service.UID) } - name = fmt.Sprintf("%s%s", prefix, service.UID) } } if len(name) > 1024 { @@ -748,6 +762,11 @@ func getSSLEnabledPorts(svc *api.Service) ([]int, error) { ports := []int{} annotation, ok := svc.Annotations[ServiceAnnotationLoadBalancerSSLPorts] if !ok || annotation == "" { + if svc.Annotations[ServiceAnnotationLoadbalancerBackendSetCABundle] != "" { + for _, servicePort := range svc.Spec.Ports { + ports = append(ports, int(servicePort.Port)) + } + } return ports, nil } diff --git a/pkg/cloudprovider/providers/oci/load_balancer_util_test.go b/pkg/cloudprovider/providers/oci/load_balancer_util_test.go index 1f0bcade0f..4edc4f9ae0 100644 --- a/pkg/cloudprovider/providers/oci/load_balancer_util_test.go +++ b/pkg/cloudprovider/providers/oci/load_balancer_util_test.go @@ -19,11 +19,13 @@ import ( "fmt" "os" "reflect" + "strings" "testing" "go.uber.org/zap" api "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" "github.com/oracle/oci-cloud-controller-manager/pkg/oci/client" "github.com/oracle/oci-go-sdk/v65/common" @@ -2548,6 +2550,56 @@ func TestGetSSLConfigurationChanges(t *testing.T) { }, expected: []string{}, }, + { + name: "TrustedCertificateAuthorityIds unchanged", + desired: client.GenericSslConfigurationDetails{ + TrustedCertificateAuthorityIds: []string{"ocid1.cabundle.oc1..a"}, + }, + actual: client.GenericSslConfigurationDetails{ + TrustedCertificateAuthorityIds: []string{"ocid1.cabundle.oc1..a"}, + }, + expected: []string{}, + }, + { + name: "TrustedCertificateAuthorityIds changed", + desired: client.GenericSslConfigurationDetails{ + TrustedCertificateAuthorityIds: []string{"ocid1.cabundle.oc1..b"}, + }, + actual: client.GenericSslConfigurationDetails{ + TrustedCertificateAuthorityIds: []string{"ocid1.cabundle.oc1..a"}, + }, + expected: []string{ + fmt.Sprintf(changeFmtStr, "Listener:SSLConfiguration:TrustedCertificateAuthorityIds", "ocid1.cabundle.oc1..a", "ocid1.cabundle.oc1..b"), + }, + }, + { + name: "TrustedCertificateAuthorityIds removed", + desired: client.GenericSslConfigurationDetails{ + TrustedCertificateAuthorityIds: nil, + }, + actual: client.GenericSslConfigurationDetails{ + TrustedCertificateAuthorityIds: []string{"ocid1.cabundle.oc1..a"}, + }, + expected: []string{ + fmt.Sprintf(changeFmtStr, "Listener:SSLConfiguration:TrustedCertificateAuthorityIds", "ocid1.cabundle.oc1..a", ""), + }, + }, + { + name: "CA bundle only with OCI-generated defaults (No drift)", + desired: client.GenericSslConfigurationDetails{ + VerifyDepth: common.Int(1), + VerifyPeerCertificate: common.Bool(false), + TrustedCertificateAuthorityIds: []string{"ocid1.cabundle.oc1..a"}, + }, + actual: client.GenericSslConfigurationDetails{ + VerifyDepth: common.Int(1), + VerifyPeerCertificate: common.Bool(false), + CipherSuiteName: common.String("oci-tls-12-ssl-cipher-suite-v3"), + Protocols: []string{"TLSv1.2"}, + TrustedCertificateAuthorityIds: []string{"ocid1.cabundle.oc1..a"}, + }, + expected: []string{}, + }, } for _, tt := range testCases { @@ -2969,3 +3021,119 @@ func Test_convertK8sIpFamiliesToOciIpVersion(t *testing.T) { }) } } + +func TestGetLoadBalancerName_CustomNameAnnotation(t *testing.T) { + svcUID := "12345678-1234-1234-1234-1234567890ab" + + t.Run("No annotation returns standard UID name", func(t *testing.T) { + svc := &api.Service{ + ObjectMeta: metav1.ObjectMeta{ + UID: types.UID(svcUID), + }, + } + got := GetLoadBalancerName(svc) + want := svcUID + if got != want { + t.Errorf("GetLoadBalancerName() = %q, want %q", got, want) + } + }) + + t.Run("Custom name annotation prepends to UID", func(t *testing.T) { + svc := &api.Service{ + ObjectMeta: metav1.ObjectMeta{ + UID: types.UID(svcUID), + Annotations: map[string]string{ + ServiceAnnotationLoadBalancerName: "my-custom-lb", + }, + }, + } + got := GetLoadBalancerName(svc) + want := "my-custom-lb-12345678-1234-1234-1234-1234567890ab" + if got != want { + t.Errorf("GetLoadBalancerName() = %q, want %q", got, want) + } + }) + + t.Run("Two Services with same custom name produce unique display names", func(t *testing.T) { + svc1 := &api.Service{ + ObjectMeta: metav1.ObjectMeta{ + UID: types.UID("uid-1111"), + Annotations: map[string]string{ + ServiceAnnotationLoadBalancerName: "shared-app-name", + }, + }, + } + svc2 := &api.Service{ + ObjectMeta: metav1.ObjectMeta{ + UID: types.UID("uid-2222"), + Annotations: map[string]string{ + ServiceAnnotationLoadBalancerName: "shared-app-name", + }, + }, + } + got1 := GetLoadBalancerName(svc1) + got2 := GetLoadBalancerName(svc2) + if got1 == got2 { + t.Errorf("GetLoadBalancerName() collision: both produced %q", got1) + } + if got1 != "shared-app-name-uid-1111" || got2 != "shared-app-name-uid-2222" { + t.Errorf("Unexpected names: got1=%q, got2=%q", got1, got2) + } + }) + + t.Run("Custom name annotation takes precedence over OCI_LOAD_BALANCER_NAME_PREFIX env var", func(t *testing.T) { + t.Setenv("OCI_LOAD_BALANCER_NAME_PREFIX", "env-prefix-") + svc := &api.Service{ + ObjectMeta: metav1.ObjectMeta{ + UID: types.UID(svcUID), + Annotations: map[string]string{ + ServiceAnnotationLoadBalancerName: "override-lb", + }, + }, + } + got := GetLoadBalancerName(svc) + want := "override-lb-12345678-1234-1234-1234-1234567890ab" + if got != want { + t.Errorf("GetLoadBalancerName() = %q, want %q", got, want) + } + }) + + t.Run("Long custom name is safely truncated at 1024 characters preserving full UID suffix", func(t *testing.T) { + longCustomName := strings.Repeat("a", 1100) + svc := &api.Service{ + ObjectMeta: metav1.ObjectMeta{ + UID: types.UID(svcUID), + Annotations: map[string]string{ + ServiceAnnotationLoadBalancerName: longCustomName, + }, + }, + } + got := GetLoadBalancerName(svc) + if len(got) != 1024 { + t.Errorf("len(GetLoadBalancerName()) = %d, want 1024", len(got)) + } + expectedSuffix := "-" + svcUID + if !strings.HasSuffix(got, expectedSuffix) { + t.Errorf("GetLoadBalancerName() = %q, does not end with expected UID suffix %q", got, expectedSuffix) + } + }) + + t.Run("NLB naming remains unchanged despite custom name annotation", func(t *testing.T) { + svc := &api.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-nlb-svc", + Namespace: "default", + UID: types.UID(svcUID), + Annotations: map[string]string{ + ServiceAnnotationLoadBalancerType: "nlb", + ServiceAnnotationLoadBalancerName: "custom-nlb-ignored", + }, + }, + } + got := GetLoadBalancerName(svc) + want := "default/my-nlb-svc/12345678-1234-1234-1234-1234567890ab" + if got != want { + t.Errorf("GetLoadBalancerName() = %q, want %q", got, want) + } + }) +}