Skip to content

Commit

Permalink
add delete-protection-enabled annotation for LB preservation on Ingre…
Browse files Browse the repository at this point in the history
…ssClass delete

- Add a new annotation to preserve LB on IngressClass delete - we clear NSG IDs, WAF, and default_ingress BackendSet associated with the LB
- Update UpdateNetworkSecurityGroups method to accept parameters instead of a request object to avoid code duplication
  • Loading branch information
piyush-tiwari committed Oct 3, 2024
1 parent d99a88f commit 4203ca3
Show file tree
Hide file tree
Showing 7 changed files with 178 additions and 22 deletions.
35 changes: 35 additions & 0 deletions GettingStarted.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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).
Expand Down Expand Up @@ -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.
Expand Down
63 changes: 50 additions & 13 deletions pkg/controllers/ingressclass/ingressclass.go
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ func (c *Controller) checkForIngressClassParameterUpdates(ctx context.Context, l
}

func (c *Controller) checkForNetworkSecurityGroupsUpdate(ctx context.Context, ic *networkingv1.IngressClass) error {
lb, etag, err := c.getLoadBalancer(ctx, ic)
lb, _, err := c.getLoadBalancer(ctx, ic)
if err != nil {
return err
}
Expand All @@ -427,31 +427,68 @@ func (c *Controller) checkForNetworkSecurityGroupsUpdate(ctx context.Context, ic
return nil
}

req := ociloadbalancer.UpdateNetworkSecurityGroupsRequest{
LoadBalancerId: lb.Id,
IfMatch: common.String(etag),
UpdateNetworkSecurityGroupsDetails: ociloadbalancer.UpdateNetworkSecurityGroupsDetails{
NetworkSecurityGroupIds: nsgIdsFromSpec,
},
}
klog.Infof("Update lb nsg ids request: %s", util.PrettyPrint(req))

_, err = wrapperClient.GetLbClient().UpdateNetworkSecurityGroups(context.Background(), req)
_, 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
}

Expand Down
35 changes: 35 additions & 0 deletions pkg/controllers/ingressclass/ingressclass_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,41 @@ func TestDeleteIngressClass(t *testing.T) {
Expect(err).Should(BeNil())
}

func TestClearLoadBalancerWhenLBFound(t *testing.T) {
RegisterTestingT(t)
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

ingressClassList := util.GetIngressClassListWithLBSet("id")
ingressClassList.Items[0].Annotations[util.IngressClassFireWallIdAnnotation] = "firewallId"
ingressClassList.Items[0].Annotations[util.IngressClassNetworkSecurityGroupIdsAnnotation] = "nsgId"
c := inits(ctx, ingressClassList)
err := c.clearLoadBalancer(getContextWithClient(c, ctx), &ingressClassList.Items[0])
Expect(err).Should(BeNil())
}

func TestClearLoadBalancerWhenLBNotFound(t *testing.T) {
RegisterTestingT(t)
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

ingressClassList := util.GetIngressClassListWithLBSet("notfound")
c := inits(ctx, ingressClassList)
err := c.clearLoadBalancer(getContextWithClient(c, ctx), &ingressClassList.Items[0])
Expect(err).Should(BeNil())
}

func TestClearLoadBalancerWhenNetworkError(t *testing.T) {
RegisterTestingT(t)
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

ingressClassList := util.GetIngressClassListWithLBSet("networkerror")
c := inits(ctx, ingressClassList)
err := c.clearLoadBalancer(getContextWithClient(c, ctx), &ingressClassList.Items[0])
Expect(err).ShouldNot(BeNil())
}

func TestDeleteLoadBalancer(t *testing.T) {
RegisterTestingT(t)
ctx, cancel := context.WithCancel(context.Background())
Expand Down
14 changes: 13 additions & 1 deletion pkg/loadbalancer/loadbalancer.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,19 @@ func (lbc *LoadBalancerClient) GetBackendSetHealth(ctx context.Context, lbID str
return &resp.BackendSetHealth, nil
}

func (lbc *LoadBalancerClient) UpdateNetworkSecurityGroups(ctx context.Context, req loadbalancer.UpdateNetworkSecurityGroupsRequest) (loadbalancer.UpdateNetworkSecurityGroupsResponse, error) {
func (lbc *LoadBalancerClient) UpdateNetworkSecurityGroups(ctx context.Context, lbId string, nsgIds []string) (loadbalancer.UpdateNetworkSecurityGroupsResponse, error) {
_, etag, err := lbc.GetLoadBalancer(ctx, lbId)

req := loadbalancer.UpdateNetworkSecurityGroupsRequest{
LoadBalancerId: common.String(lbId),
IfMatch: common.String(etag),
UpdateNetworkSecurityGroupsDetails: loadbalancer.UpdateNetworkSecurityGroupsDetails{
NetworkSecurityGroupIds: nsgIds,
},
}

klog.Infof("Update LB NSG IDs request: %s", util.PrettyPrint(req))

resp, err := lbc.LbClient.UpdateNetworkSecurityGroups(ctx, req)
if err != nil {
return resp, err
Expand Down
9 changes: 1 addition & 8 deletions pkg/loadbalancer/loadbalancer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,15 +208,8 @@ func TestLoadBalancerClient_UpdateListener(t *testing.T) {
func TestLoadBalancerClient_UpdateNetworkSecurityGroups(t *testing.T) {
RegisterTestingT(t)
loadBalancerClient := setupLBClient()
req := ociloadbalancer.UpdateNetworkSecurityGroupsRequest{
LoadBalancerId: common.String("id"),
UpdateNetworkSecurityGroupsDetails: ociloadbalancer.UpdateNetworkSecurityGroupsDetails{
NetworkSecurityGroupIds: []string{"id1", "id2"},
},
OpcRetryToken: common.String("token"),
}

_, err := loadBalancerClient.UpdateNetworkSecurityGroups(context.TODO(), req)
_, err := loadBalancerClient.UpdateNetworkSecurityGroups(context.TODO(), "id", []string{"id1", "id2"})
Expect(err).To(BeNil())
}

Expand Down
18 changes: 18 additions & 0 deletions pkg/util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ const (
IngressClassWafPolicyAnnotation = "oci-native-ingress.oraclecloud.com/waf-policy-ocid"
IngressClassFireWallIdAnnotation = "oci-native-ingress.oraclecloud.com/firewall-id"
IngressClassNetworkSecurityGroupIdsAnnotation = "oci-native-ingress.oraclecloud.com/network-security-group-ids"
IngressClassDeleteProtectionEnabledAnnotation = "oci-native-ingress.oraclecloud.com/delete-protection-enabled"

IngressHealthCheckProtocolAnnotation = "oci-native-ingress.oraclecloud.com/healthcheck-protocol"
IngressHealthCheckPortAnnotation = "oci-native-ingress.oraclecloud.com/healthcheck-port"
Expand Down Expand Up @@ -171,6 +172,23 @@ func GetIngressClassNetworkSecurityGroupIds(ic *networkingv1.IngressClass) []str
return networkSecurityGroupIds
}

func GetIngressClassDeleteProtectionEnabled(ic *networkingv1.IngressClass) bool {
annotation := IngressClassDeleteProtectionEnabledAnnotation
value, ok := ic.Annotations[annotation]

if !ok || strings.TrimSpace(value) == "" {
return false
}

result, err := strconv.ParseBool(value)
if err != nil {
klog.Errorf("Error parsing value %s for flag %s as boolean. Setting the default value as 'false'", value, annotation)
return false
}

return result
}

func GetIngressProtocol(i *networkingv1.Ingress) string {
protocol, ok := i.Annotations[IngressProtocolAnnotation]
if !ok {
Expand Down
26 changes: 26 additions & 0 deletions pkg/util/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,32 @@ func TestGetIngressClassNetworkSecurityGroupIds(t *testing.T) {
Should(Equal([]string{"id1", "id2", "id3", "id4"}))
}

func TestGetIngressClassDeleteProtectionEnabled(t *testing.T) {
RegisterTestingT(t)

getIngressClassWithDeleteProtectionEnabledAnnotation := func(annotation string) *networkingv1.IngressClass {
return &networkingv1.IngressClass{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{IngressClassDeleteProtectionEnabledAnnotation: annotation},
},
}
}

ingressClassWithNoAnnotation := &networkingv1.IngressClass{
ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{}},
}
ingressClassWithEmptyAnnotation := getIngressClassWithDeleteProtectionEnabledAnnotation("")
ingressClassWithProtectionEnabled := getIngressClassWithDeleteProtectionEnabledAnnotation("true")
ingressClassWithProtectionDisabled := getIngressClassWithDeleteProtectionEnabledAnnotation("false")
ingressClassWithWrongAnnotation := getIngressClassWithDeleteProtectionEnabledAnnotation("n/a")

Expect(GetIngressClassDeleteProtectionEnabled(ingressClassWithNoAnnotation)).Should(BeFalse())
Expect(GetIngressClassDeleteProtectionEnabled(ingressClassWithEmptyAnnotation)).Should(BeFalse())
Expect(GetIngressClassDeleteProtectionEnabled(ingressClassWithProtectionEnabled)).Should(BeTrue())
Expect(GetIngressClassDeleteProtectionEnabled(ingressClassWithProtectionDisabled)).Should(BeFalse())
Expect(GetIngressClassDeleteProtectionEnabled(ingressClassWithWrongAnnotation)).Should(BeFalse())
}

func TestGetIngressClassLoadBalancerId(t *testing.T) {
RegisterTestingT(t)
lbId := "lbId"
Expand Down

0 comments on commit 4203ca3

Please sign in to comment.