Sync with internal repo

GettingStarted.md

@@ -36,6 +36,8 @@ The native ingress controller itself is lightweight process and pushes all the r
     + [Web Firewall Integration](#web-firewall-integration)
     + [Ingress Level HTTP(S) Listener Ports](#ingress-level-https-listener-ports)
     + [TCP Listener Support](#tcp-listener-support)
+    + [Network Security Groups Support](#network-security-groups-support)
+    + [Load Balancer Preservation on `IngressClass` delete](#load-balancer-preservation-on-ingressclass-delete)
   * [Dependency management](#dependency-management)
     + [How to introduce new modules or upgrade existing ones?](#how-to-introduce-new-modules-or-upgrade-existing-ones)
   * [Known Issues](#known-issues)
@@ -50,6 +52,7 @@ Currently supported kubernetes versions are:
 - 1.27
 - 1.28
 - 1.29
+- 1.30
 
 We set up the cluster with either native pod networking or flannel CNI and update the security rules. 
 The documentation for NPN : [Doc Ref](https://docs.oracle.com/en-us/iaas/Content/ContEng/Concepts/contengpodnetworking_topic-OCI_CNI_plugin.htm).
@@ -603,6 +606,38 @@ spec:
                   number: 8081
 ```
 
+### Network Security Groups Support
+Users can use the `IngressClass` resource annotation `oci-native-ingress.oraclecloud.com/network-security-group-ids` to supply
+a comma separated list of Network Security Group OCIDs.
+The supplied NSGs will be associated with the LB associated with the `IngressClass`.
+
+Example:
+```yaml
+apiVersion: networking.k8s.io/v1
+kind: IngressClass
+metadata:
+ annotations:
+   oci-native-ingress.oraclecloud.com/network-security-group-ids: ocid1.networksecuritygroup.oc1.abc,ocid1.networksecuritygroup.oc1.xyz
+```
+
+### Load Balancer Preservation on `IngressClass` delete
+If you want the Load Balancer associated with an `IngressClass` resource to be preserved after `IngressClass` is deleted,
+set the annotation `oci-native-ingress.oraclecloud.com/delete-protection-enabled` annotation to `"true"`.
+This annotation defaults to `"false"` when not specified or empty.
+
+OCI Native Ingress Controller will aim to leave the LB in a 'blank' state - clear all NSG associated with the LB,
+delete the Web App Firewall associated with the LB if any, and delete the `default_ingress` BackendSet when the `IngressClass` is deleted with this annotation set to true.
+Please note that users should first delete all `Ingress` resources associated with this `IngressClass` first, or orphaned resources like Listeners, BackendSets, etc. will
+still be present on the LB after the `IngressClass` is deleted
+
+Example:
+```yaml
+apiVersion: networking.k8s.io/v1
+kind: IngressClass
+metadata:
+ annotations:
+   oci-native-ingress.oraclecloud.com/delete-protection-enabled: "true"
+```
 
 ### Dependency management
 Module [vendoring](https://go.dev/ref/mod#vendoring) is used to manage 3d-party modules in the project.

pkg/controllers/backend/backend_test.go

@@ -295,6 +295,10 @@ func (m MockLoadBalancerClient) UpdateLoadBalancerShape(ctx context.Context, req
 	return ociloadbalancer.UpdateLoadBalancerShapeResponse{}, nil
 }
 
+func (m MockLoadBalancerClient) UpdateNetworkSecurityGroups(ctx context.Context, request ociloadbalancer.UpdateNetworkSecurityGroupsRequest) (ociloadbalancer.UpdateNetworkSecurityGroupsResponse, error) {
+	return ociloadbalancer.UpdateNetworkSecurityGroupsResponse{}, nil
+}
+
 func (m MockLoadBalancerClient) GetLoadBalancer(ctx context.Context, request ociloadbalancer.GetLoadBalancerRequest) (ociloadbalancer.GetLoadBalancerResponse, error) {
 	res := util.SampleLoadBalancerResponse()
 	return res, nil

pkg/controllers/ingress/ingress.go

@@ -522,7 +522,7 @@ func syncListener(ctx context.Context, namespace string, stateStore *state.State
 		}
 
 		if sslConfig != nil {
-			if !reflect.DeepEqual(listener.SslConfiguration.CertificateIds, sslConfig.CertificateIds) {
+			if listener.SslConfiguration == nil || !reflect.DeepEqual(listener.SslConfiguration.CertificateIds, sslConfig.CertificateIds) {
 				klog.Infof("SSL config for listener update is %s", util.PrettyPrint(sslConfig))
 				needsUpdate = true
 			}
@@ -579,11 +579,10 @@ func syncBackendSet(ctx context.Context, ingress *networkingv1.Ingress, lbID str
 	if err != nil {
 		return err
 	}
-	if sslConfig != nil {
-		if bs.SslConfiguration == nil || !reflect.DeepEqual(bs.SslConfiguration.TrustedCertificateAuthorityIds, sslConfig.TrustedCertificateAuthorityIds) {
-			klog.Infof("SSL config for backend set %s update is %s", *bs.Name, util.PrettyPrint(sslConfig))
-			needsUpdate = true
-		}
+
+	if backendSetSslConfigNeedsUpdate(sslConfig, &bs) {
+		klog.Infof("SSL config for backend set %s update is %s", *bs.Name, util.PrettyPrint(sslConfig))
+		needsUpdate = true
 	}
 
 	healthChecker := stateStore.GetBackendSetHealthChecker(*bs.Name)
@@ -601,7 +600,7 @@ func syncBackendSet(ctx context.Context, ingress *networkingv1.Ingress, lbID str
 	}
 
 	if needsUpdate {
-		err = wrapperClient.GetLbClient().UpdateBackendSet(context.TODO(), lb.Id, etag, bs, nil, sslConfig, healthChecker, &policy)
+		err = wrapperClient.GetLbClient().UpdateBackendSetDetails(context.TODO(), *lb.Id, etag, &bs, sslConfig, healthChecker, policy)
 		if err != nil {
 			return err
 		}

pkg/controllers/ingress/ingress_test.go

@@ -225,6 +225,10 @@ func (m MockLoadBalancerClient) UpdateLoadBalancerShape(ctx context.Context, req
 	return ociloadbalancer.UpdateLoadBalancerShapeResponse{}, nil
 }
 
+func (m MockLoadBalancerClient) UpdateNetworkSecurityGroups(ctx context.Context, request ociloadbalancer.UpdateNetworkSecurityGroupsRequest) (ociloadbalancer.UpdateNetworkSecurityGroupsResponse, error) {
+	return ociloadbalancer.UpdateNetworkSecurityGroupsResponse{}, nil
+}
+
 func (m MockLoadBalancerClient) CreateLoadBalancer(ctx context.Context, request ociloadbalancer.CreateLoadBalancerRequest) (ociloadbalancer.CreateLoadBalancerResponse, error) {
 	return ociloadbalancer.CreateLoadBalancerResponse{}, nil
 }

pkg/controllers/ingress/util.go

@@ -429,3 +429,17 @@ func CreateOrGetCaBundleForBackendSet(namespace string, secretName string, compa
 func isTrustAuthorityCaBundle(id string) bool {
 	return strings.Contains(id, "cabundle")
 }
+
+func backendSetSslConfigNeedsUpdate(calculatedConfig *ociloadbalancer.SslConfigurationDetails,
+	currentBackendSet *ociloadbalancer.BackendSet) bool {
+	if calculatedConfig == nil && currentBackendSet.SslConfiguration != nil {
+		return true
+	}
+
+	if calculatedConfig != nil && (currentBackendSet.SslConfiguration == nil ||
+		!reflect.DeepEqual(currentBackendSet.SslConfiguration.TrustedCertificateAuthorityIds, calculatedConfig.TrustedCertificateAuthorityIds)) {
+		return true
+	}
+
+	return false
+}

pkg/controllers/ingress/util_test.go

@@ -367,6 +367,34 @@ intermediatecert
 	Expect(err).To(HaveOccurred())
 }
 
+func TestBackendSetSslConfigNeedsUpdate(t *testing.T) {
+	RegisterTestingT(t)
+
+	caBundleId1 := []string{"caCert1"}
+	caBundleId2 := []string{"caCert2"}
+
+	backendSetWithNilSslConfig := &ociloadbalancer.BackendSet{}
+	presentBackendSet1 := &ociloadbalancer.BackendSet{
+		SslConfiguration: &ociloadbalancer.SslConfiguration{
+			TrustedCertificateAuthorityIds: caBundleId1,
+		},
+	}
+	presentBackendSet2 := &ociloadbalancer.BackendSet{
+		SslConfiguration: &ociloadbalancer.SslConfiguration{
+			TrustedCertificateAuthorityIds: caBundleId2,
+		},
+	}
+	calculatedConfig1 := &ociloadbalancer.SslConfigurationDetails{
+		TrustedCertificateAuthorityIds: caBundleId1,
+	}
+
+	Expect(backendSetSslConfigNeedsUpdate(nil, backendSetWithNilSslConfig)).To(BeFalse())
+	Expect(backendSetSslConfigNeedsUpdate(nil, presentBackendSet1)).To(BeTrue())
+	Expect(backendSetSslConfigNeedsUpdate(calculatedConfig1, presentBackendSet1)).To(BeFalse())
+	Expect(backendSetSslConfigNeedsUpdate(calculatedConfig1, presentBackendSet2)).To(BeTrue())
+	Expect(backendSetSslConfigNeedsUpdate(calculatedConfig1, backendSetWithNilSslConfig)).To(BeTrue())
+}
+
 func generateTestCertsAndKey() (string, string, string) {
 	caCert := &x509.Certificate{
 		SerialNumber: big.NewInt(1),

pkg/controllers/ingressclass/ingressclass.go

@@ -207,33 +207,35 @@ func (c *Controller) sync(key string) error {
 	return nil
 }
 
-func (c *Controller) getLoadBalancer(ctx context.Context, ic *networkingv1.IngressClass) (*ociloadbalancer.LoadBalancer, error) {
+func (c *Controller) getLoadBalancer(ctx context.Context, ic *networkingv1.IngressClass) (*ociloadbalancer.LoadBalancer, string, error) {
 	lbID := util.GetIngressClassLoadBalancerId(ic)
 	if lbID == "" {
-		klog.Errorf("LB id not set for ingressClass: %s", ic.Name)
-		return nil, nil // LoadBalancer ID not set, Trigger new LB creation
+		klog.Infof("LB id not set for ingressClass: %s", ic.Name)
+		return nil, "", nil // LoadBalancer ID not set, Trigger new LB creation
 	}
 	wrapperClient, ok := ctx.Value(util.WrapperClient).(*client.WrapperClient)
 	if !ok {
-		return nil, fmt.Errorf(util.OciClientNotFoundInContextError)
+		return nil, "", fmt.Errorf(util.OciClientNotFoundInContextError)
 	}
-	lb, _, err := wrapperClient.GetLbClient().GetLoadBalancer(context.TODO(), lbID)
+
+	lb, etag, err := wrapperClient.GetLbClient().GetLoadBalancer(context.TODO(), lbID)
 	if err != nil {
 		klog.Errorf("Error while fetching LB %s for ingressClass: %s, err: %s", lbID, ic.Name, err.Error())
 
 		// Check if Service error 404, then ignore it since LB is not found.
 		svcErr, ok := common.IsServiceError(err)
 		if ok && svcErr.GetHTTPStatusCode() == 404 {
-			return nil, nil // Redirect new LB creation
+			return nil, "", nil // Redirect new LB creation
 		}
-		return nil, err
+		return nil, "", err
 	}
-	return lb, nil
+
+	return lb, etag, nil
 }
 
 func (c *Controller) ensureLoadBalancer(ctx context.Context, ic *networkingv1.IngressClass) error {
 
-	lb, err := c.getLoadBalancer(ctx, ic)
+	lb, etag, err := c.getLoadBalancer(ctx, ic)
 	if err != nil {
 		return err
 	}
@@ -262,11 +264,12 @@ func (c *Controller) ensureLoadBalancer(ctx context.Context, ic *networkingv1.In
 		klog.V(2).InfoS("Creating load balancer for ingress class", "ingressClass", ic.Name)
 
 		createDetails := ociloadbalancer.CreateLoadBalancerDetails{
-			CompartmentId: compartmentId,
-			DisplayName:   common.String(util.GetIngressClassLoadBalancerName(ic, icp)),
-			ShapeName:     common.String("flexible"),
-			SubnetIds:     []string{util.GetIngressClassSubnetId(icp, c.defaultSubnetId)},
-			IsPrivate:     common.Bool(icp.Spec.IsPrivate),
+			CompartmentId:           compartmentId,
+			DisplayName:             common.String(util.GetIngressClassLoadBalancerName(ic, icp)),
+			ShapeName:               common.String("flexible"),
+			SubnetIds:               []string{util.GetIngressClassSubnetId(icp, c.defaultSubnetId)},
+			IsPrivate:               common.Bool(icp.Spec.IsPrivate),
+			NetworkSecurityGroupIds: util.GetIngressClassNetworkSecurityGroupIds(ic),
 			BackendSets: map[string]ociloadbalancer.BackendSetDetails{
 				util.DefaultBackendSetName: {
 					Policy: common.String("LEAST_CONNECTIONS"),
@@ -300,7 +303,15 @@ func (c *Controller) ensureLoadBalancer(ctx context.Context, ic *networkingv1.In
 			return err
 		}
 	} else {
-		c.checkForIngressClassParameterUpdates(ctx, lb, ic, icp)
+		err = c.checkForIngressClassParameterUpdates(ctx, lb, ic, icp, etag)
+		if err != nil {
+			return err
+		}
+
+		err = c.checkForNetworkSecurityGroupsUpdate(ctx, ic)
+		if err != nil {
+			return err
+		}
 	}
 
 	if *lb.Id != util.GetIngressClassLoadBalancerId(ic) {
@@ -342,7 +353,8 @@ func (c *Controller) setupWebApplicationFirewall(ctx context.Context, ic *networ
 	return nil
 }
 
-func (c *Controller) checkForIngressClassParameterUpdates(ctx context.Context, lb *ociloadbalancer.LoadBalancer, ic *networkingv1.IngressClass, icp *v1beta1.IngressClassParameters) error {
+func (c *Controller) checkForIngressClassParameterUpdates(ctx context.Context, lb *ociloadbalancer.LoadBalancer,
+	ic *networkingv1.IngressClass, icp *v1beta1.IngressClassParameters, etag string) error {
 	// check LoadBalancerName AND  MinBandwidthMbps ,MaxBandwidthMbps
 	displayName := util.GetIngressClassLoadBalancerName(ic, icp)
 	wrapperClient, ok := ctx.Value(util.WrapperClient).(*client.WrapperClient)
@@ -377,11 +389,11 @@ func (c *Controller) checkForIngressClassParameterUpdates(ctx context.Context, l
 
 		req := ociloadbalancer.UpdateLoadBalancerShapeRequest{
 			LoadBalancerId: lb.Id,
+			IfMatch:        common.String(etag),
 			UpdateLoadBalancerShapeDetails: ociloadbalancer.UpdateLoadBalancerShapeDetails{
 				ShapeName:    common.String("flexible"),
 				ShapeDetails: shapeDetails,
 			},
-			OpcRetryToken: common.String(fmt.Sprintf("update-lb-shape-%s", ic.UID)),
 		}
 		klog.Infof("Update lb shape request: %s", util.PrettyPrint(req))
 		_, err := wrapperClient.GetLbClient().UpdateLoadBalancerShape(context.Background(), req)
@@ -393,18 +405,90 @@ func (c *Controller) checkForIngressClassParameterUpdates(ctx context.Context, l
 	return nil
 }
 
+func (c *Controller) checkForNetworkSecurityGroupsUpdate(ctx context.Context, ic *networkingv1.IngressClass) error {
+	lb, _, err := c.getLoadBalancer(ctx, ic)
+	if err != nil {
+		return err
+	}
+
+	wrapperClient, ok := ctx.Value(util.WrapperClient).(*client.WrapperClient)
+	if !ok {
+		return fmt.Errorf(util.OciClientNotFoundInContextError)
+	}
+
+	nsgIdsFromSpec := util.GetIngressClassNetworkSecurityGroupIds(ic)
+
+	/*
+		Only check if desired and actual slices have the same elements, ignoring order and duplicates
+		We don't check if lb.NetworkSecurityGroupIds is nil since util.StringSlicesHaveSameElements returns true if
+		one argument is nil and the other is empty.
+	*/
+	if util.StringSlicesHaveSameElements(nsgIdsFromSpec, lb.NetworkSecurityGroupIds) {
+		return nil
+	}
+
+	_, err = wrapperClient.GetLbClient().UpdateNetworkSecurityGroups(context.Background(), *lb.Id, nsgIdsFromSpec)
+	return err
+}
+
 func (c *Controller) deleteIngressClass(ctx context.Context, ic *networkingv1.IngressClass) error {
+	if util.GetIngressClassDeleteProtectionEnabled(ic) {
+		err := c.clearLoadBalancer(ctx, ic)
+		if err != nil {
+			return err
+		}
+	} else {
+		err := c.deleteLoadBalancer(ctx, ic)
+		if err != nil {
+			return err
+		}
+	}
 
-	err := c.deleteLoadBalancer(ctx, ic)
+	err := c.deleteFinalizer(ctx, ic)
 	if err != nil {
 		return err
 	}
 
-	err = c.deleteFinalizer(ctx, ic)
+	return nil
+}
+
+// clearLoadBalancer clears the default_ingress backend, NSG attachment, and WAF firewall from the LB
+func (c *Controller) clearLoadBalancer(ctx context.Context, ic *networkingv1.IngressClass) error {
+	lb, _, err := c.getLoadBalancer(ctx, ic)
 	if err != nil {
 		return err
 	}
 
+	if lb == nil {
+		klog.Infof("Tried to clear LB for ic %s/%s, but it is deleted", ic.Namespace, ic.Name)
+		return nil
+	}
+
+	wrapperClient, ok := ctx.Value(util.WrapperClient).(*client.WrapperClient)
+	if !ok {
+		return fmt.Errorf(util.OciClientNotFoundInContextError)
+	}
+
+	fireWallId := util.GetIngressClassFireWallId(ic)
+	if fireWallId != "" {
+		wrapperClient.GetWafClient().DeleteWebAppFirewallWithId(fireWallId)
+	}
+
+	nsgIds := util.GetIngressClassNetworkSecurityGroupIds(ic)
+	if len(nsgIds) > 0 {
+		_, err = wrapperClient.GetLbClient().UpdateNetworkSecurityGroups(context.Background(), *lb.Id, make([]string, 0))
+		if err != nil {
+			klog.Errorf("While clearing LB %s, cannot clear NSG IDs due to %s, will proceed with IngressClass deletion for %s/%s",
+				*lb.Id, err.Error(), ic.Namespace, ic.Name)
+		}
+	}
+
+	err = wrapperClient.GetLbClient().DeleteBackendSet(context.Background(), *lb.Id, util.DefaultBackendSetName)
+	if err != nil {
+		klog.Errorf("While clearing LB %s, cannot clear BackendSet %s due to %s, will proceed with IngressClass deletion for %s/%s",
+			*lb.Id, util.DefaultBackendSetName, err.Error(), ic.Namespace, ic.Name)
+	}
+
 	return nil
 }