diff --git a/pkg/controllers/ingressclass/ingressclass.go b/pkg/controllers/ingressclass/ingressclass.go index 31704310..16110b87 100644 --- a/pkg/controllers/ingressclass/ingressclass.go +++ b/pkg/controllers/ingressclass/ingressclass.go @@ -14,6 +14,7 @@ import ( "fmt" coreinformers "k8s.io/client-go/informers/core/v1" corelisters "k8s.io/client-go/listers/core/v1" + "reflect" "time" "github.com/oracle/oci-native-ingress-controller/pkg/client" @@ -234,8 +235,7 @@ func (c *Controller) getLoadBalancer(ctx context.Context, ic *networkingv1.Ingre } func (c *Controller) ensureLoadBalancer(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 } @@ -255,55 +255,13 @@ func (c *Controller) ensureLoadBalancer(ctx context.Context, ic *networkingv1.In } } - wrapperClient, ok := ctx.Value(util.WrapperClient).(*client.WrapperClient) - if !ok { - return fmt.Errorf(util.OciClientNotFoundInContextError) - } - compartmentId := common.String(util.GetIngressClassCompartmentId(icp, c.defaultCompartmentId)) if lb == nil { - 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), - NetworkSecurityGroupIds: util.GetIngressClassNetworkSecurityGroupIds(ic), - BackendSets: map[string]ociloadbalancer.BackendSetDetails{ - util.DefaultBackendSetName: { - Policy: common.String("LEAST_CONNECTIONS"), - HealthChecker: &ociloadbalancer.HealthCheckerDetails{ - Protocol: common.String("TCP"), - }, - }, - }, - FreeformTags: map[string]string{OnicResource: "loadbalancer"}, - } - - if icp.Spec.ReservedPublicAddressId != "" { - createDetails.ReservedIps = []ociloadbalancer.ReservedIp{{Id: common.String(icp.Spec.ReservedPublicAddressId)}} - } - - createDetails.ShapeDetails = &ociloadbalancer.ShapeDetails{ - MinimumBandwidthInMbps: common.Int(icp.Spec.MinBandwidthMbps), - MaximumBandwidthInMbps: common.Int(icp.Spec.MaxBandwidthMbps), - } - - createLbRequest := ociloadbalancer.CreateLoadBalancerRequest{ - // Use UID as retry token so multiple requests in 24 hours won't recreate the same LoadBalancer, - // but recreate of the IngressClass will trigger an LB within 24 hours. - // If you used ingress class name it would disallow creation of more LB's even in different clusters potentially. - OpcRetryToken: common.String(fmt.Sprintf("create-lb-%s", ic.UID)), - CreateLoadBalancerDetails: createDetails, - } - klog.Infof("Create lb request: %s", util.PrettyPrint(createLbRequest)) - lb, err = wrapperClient.GetLbClient().CreateLoadBalancer(context.Background(), createLbRequest) + lb, err = c.createLoadBalancer(ctx, ic, icp) if err != nil { - return err + return fmt.Errorf("unable to create LoadBalancer for IngressClass %s: %w", ic.Name, err) } } else { - err = c.checkForIngressClassParameterUpdates(ctx, lb, ic, icp, etag) + err = c.checkForIngressClassParameterUpdates(ctx, ic, icp) if err != nil { return err } @@ -314,6 +272,12 @@ func (c *Controller) ensureLoadBalancer(ctx context.Context, ic *networkingv1.In } } + wrapperClient, ok := ctx.Value(util.WrapperClient).(*client.WrapperClient) + if !ok { + return fmt.Errorf(util.OciClientNotFoundInContextError) + } + compartmentId := common.String(util.GetIngressClassCompartmentId(icp, c.defaultCompartmentId)) + if *lb.Id != util.GetIngressClassLoadBalancerId(ic) { klog.InfoS("Adding load balancer id to ingress class", "lbId", *lb.Id, "ingressClass", klog.KObj(ic)) patchError, done := util.PatchIngressClassWithAnnotation(wrapperClient.GetK8Client(), ic, util.IngressClassLoadBalancerIdAnnotation, *lb.Id) @@ -334,6 +298,68 @@ func (c *Controller) ensureLoadBalancer(ctx context.Context, ic *networkingv1.In return nil } +func (c *Controller) createLoadBalancer(ctx context.Context, ic *networkingv1.IngressClass, + icp *v1beta1.IngressClassParameters) (*ociloadbalancer.LoadBalancer, error) { + klog.V(2).InfoS("Creating load balancer for ingress class", "ingressClass", ic.Name) + compartmentId := common.String(util.GetIngressClassCompartmentId(icp, c.defaultCompartmentId)) + definedTags, err := util.GetIngressClassDefinedTags(ic) + if err != nil { + return nil, err + } + freeformTags, err := util.GetIngressClassFreeformTags(ic) + if err != nil { + return nil, err + } + + 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), + NetworkSecurityGroupIds: util.GetIngressClassNetworkSecurityGroupIds(ic), + BackendSets: map[string]ociloadbalancer.BackendSetDetails{ + util.DefaultBackendSetName: { + Policy: common.String("LEAST_CONNECTIONS"), + HealthChecker: &ociloadbalancer.HealthCheckerDetails{ + Protocol: common.String("TCP"), + }, + }, + }, + FreeformTags: freeformTags, + DefinedTags: definedTags, + } + + if icp.Spec.ReservedPublicAddressId != "" { + createDetails.ReservedIps = []ociloadbalancer.ReservedIp{{Id: common.String(icp.Spec.ReservedPublicAddressId)}} + } + + createDetails.ShapeDetails = &ociloadbalancer.ShapeDetails{ + MinimumBandwidthInMbps: common.Int(icp.Spec.MinBandwidthMbps), + MaximumBandwidthInMbps: common.Int(icp.Spec.MaxBandwidthMbps), + } + + createLbRequest := ociloadbalancer.CreateLoadBalancerRequest{ + // Use UID as retry token so multiple requests in 24 hours won't recreate the same LoadBalancer, + // but recreate of the IngressClass will trigger an LB within 24 hours. + // If you used ingress class name it would disallow creation of more LB's even in different clusters potentially. + OpcRetryToken: common.String(fmt.Sprintf("create-lb-%s", ic.UID)), + CreateLoadBalancerDetails: createDetails, + } + klog.Infof("Create lb request: %s", util.PrettyPrint(createLbRequest)) + + wrapperClient, ok := ctx.Value(util.WrapperClient).(*client.WrapperClient) + if !ok { + return nil, fmt.Errorf(util.OciClientNotFoundInContextError) + } + lb, err := wrapperClient.GetLbClient().CreateLoadBalancer(context.Background(), createLbRequest) + if err != nil { + return nil, err + } + + return lb, nil +} + func (c *Controller) setupWebApplicationFirewall(ctx context.Context, ic *networkingv1.IngressClass, compartmentId *string, lbId *string) error { wrapperClient, ok := ctx.Value(util.WrapperClient).(*client.WrapperClient) if !ok { @@ -353,33 +379,43 @@ 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, etag string) error { - // check LoadBalancerName AND MinBandwidthMbps ,MaxBandwidthMbps - displayName := util.GetIngressClassLoadBalancerName(ic, icp) +func (c *Controller) checkForIngressClassParameterUpdates(ctx context.Context, ic *networkingv1.IngressClass, icp *v1beta1.IngressClassParameters) error { + lb, etag, 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) } - if *lb.DisplayName != displayName { - detail := ociloadbalancer.UpdateLoadBalancerDetails{ - DisplayName: &displayName, - } - req := ociloadbalancer.UpdateLoadBalancerRequest{ - OpcRetryToken: common.String(fmt.Sprintf("update-lb-detail-%s", ic.UID)), - UpdateLoadBalancerDetails: detail, - LoadBalancerId: lb.Id, - } + // check LoadBalancerName, Defined and Freeform tags + displayName := util.GetIngressClassLoadBalancerName(ic, icp) + definedTags, err := util.GetIngressClassDefinedTags(ic) + if err != nil { + return err + } + freeformTags, err := util.GetIngressClassFreeformTags(ic) + if err != nil { + return err + } - klog.Infof("Update lb details request: %s", util.PrettyPrint(req)) - _, err := wrapperClient.GetLbClient().UpdateLoadBalancer(context.Background(), req) + if *lb.DisplayName != displayName || !reflect.DeepEqual(lb.DefinedTags, definedTags) || !reflect.DeepEqual(lb.FreeformTags, freeformTags) { + _, err = wrapperClient.GetLbClient().UpdateLoadBalancer(context.Background(), *lb.Id, displayName, definedTags, freeformTags) if err != nil { return err } } + // refresh lb, etag information after last call + lb, etag, err = c.getLoadBalancer(ctx, ic) + if err != nil { + return err + } + + // check LB Shape if *lb.ShapeDetails.MaximumBandwidthInMbps != icp.Spec.MaxBandwidthMbps || *lb.ShapeDetails.MinimumBandwidthInMbps != icp.Spec.MinBandwidthMbps { shapeDetails := &ociloadbalancer.ShapeDetails{ @@ -460,7 +496,7 @@ func (c *Controller) clearLoadBalancer(ctx context.Context, ic *networkingv1.Ing } if lb == nil { - klog.Infof("Tried to clear LB for ic %s/%s, but it is deleted", ic.Namespace, ic.Name) + klog.Infof("Tried to clear LB for ic %s, but it is deleted", ic.Name) return nil } @@ -478,15 +514,21 @@ func (c *Controller) clearLoadBalancer(ctx context.Context, ic *networkingv1.Ing 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) + klog.Errorf("While clearing LB %s, cannot clear NSG IDs due to %s, will proceed with IngressClass deletion for %s", + *lb.Id, err.Error(), 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) + klog.Errorf("While clearing LB %s, cannot clear BackendSet %s due to %s, will proceed with IngressClass deletion for %s", + *lb.Id, util.DefaultBackendSetName, err.Error(), ic.Name) + } + + _, err = wrapperClient.GetLbClient().UpdateLoadBalancer(context.Background(), *lb.Id, *lb.DisplayName, map[string]map[string]interface{}{}, map[string]string{}) + if err != nil { + klog.Errorf("While clearing LB %s, cannot clear tags due to %s, will proceed with IngressClass deletion for %s", + *lb.Id, err.Error(), ic.Name) } return nil diff --git a/pkg/controllers/ingressclass/ingressclass_test.go b/pkg/controllers/ingressclass/ingressclass_test.go index 29a681f8..bc76be11 100644 --- a/pkg/controllers/ingressclass/ingressclass_test.go +++ b/pkg/controllers/ingressclass/ingressclass_test.go @@ -202,11 +202,8 @@ func TestCheckForIngressClassParameterUpdates(t *testing.T) { RegisterTestingT(t) ctx, cancel := context.WithCancel(context.Background()) defer cancel() - ingressClassList := util.GetIngressClassList() + ingressClassList := util.GetIngressClassListWithLBSet("id") c := inits(ctx, ingressClassList) - mockClient, err := c.client.GetClient(&MockConfigGetter{}) - Expect(err).To(BeNil()) - loadBalancer, _, _ := mockClient.GetLbClient().GetLoadBalancer(context.TODO(), "id") icp := v1beta1.IngressClassParameters{ Spec: v1beta1.IngressClassParametersSpec{ CompartmentId: "", @@ -217,7 +214,7 @@ func TestCheckForIngressClassParameterUpdates(t *testing.T) { MaxBandwidthMbps: 400, }, } - err = c.checkForIngressClassParameterUpdates(getContextWithClient(c, ctx), loadBalancer, &ingressClassList.Items[0], &icp, "etag") + err := c.checkForIngressClassParameterUpdates(getContextWithClient(c, ctx), &ingressClassList.Items[0], &icp) Expect(err).Should(BeNil()) } diff --git a/pkg/loadbalancer/loadbalancer.go b/pkg/loadbalancer/loadbalancer.go index c37b3de3..2712d32f 100644 --- a/pkg/loadbalancer/loadbalancer.go +++ b/pkg/loadbalancer/loadbalancer.go @@ -88,6 +88,9 @@ func (lbc *LoadBalancerClient) GetBackendSetHealth(ctx context.Context, lbID str func (lbc *LoadBalancerClient) UpdateNetworkSecurityGroups(ctx context.Context, lbId string, nsgIds []string) (loadbalancer.UpdateNetworkSecurityGroupsResponse, error) { _, etag, err := lbc.GetLoadBalancer(ctx, lbId) + if err != nil { + return loadbalancer.UpdateNetworkSecurityGroupsResponse{}, err + } req := loadbalancer.UpdateNetworkSecurityGroupsRequest{ LoadBalancerId: common.String(lbId), @@ -128,7 +131,24 @@ func (lbc *LoadBalancerClient) UpdateLoadBalancerShape(ctx context.Context, req return resp, err } -func (lbc *LoadBalancerClient) UpdateLoadBalancer(ctx context.Context, req loadbalancer.UpdateLoadBalancerRequest) (*loadbalancer.LoadBalancer, error) { +func (lbc *LoadBalancerClient) UpdateLoadBalancer(ctx context.Context, lbId string, displayName string, definedTags map[string]map[string]interface{}, + freeformTags map[string]string) (*loadbalancer.LoadBalancer, error) { + _, etag, err := lbc.GetLoadBalancer(ctx, lbId) + if err != nil { + return nil, err + } + + req := loadbalancer.UpdateLoadBalancerRequest{ + LoadBalancerId: common.String(lbId), + IfMatch: common.String(etag), + UpdateLoadBalancerDetails: loadbalancer.UpdateLoadBalancerDetails{ + DisplayName: common.String(displayName), + DefinedTags: definedTags, + FreeformTags: freeformTags, + }, + } + + klog.Infof("Update lb details request: %s", util.PrettyPrint(req)) resp, err := lbc.LbClient.UpdateLoadBalancer(ctx, req) if err != nil { return nil, err diff --git a/pkg/util/util.go b/pkg/util/util.go index e8cd28f7..d4088992 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -68,6 +68,8 @@ const ( 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" + IngressClassDefinedTagsAnnotation = "oci-native-ingress.oraclecloud.com/defined-tags" + IngressClassFreeformTagsAnnotation = "oci-native-ingress.oraclecloud.com/freeform-tags" IngressHealthCheckProtocolAnnotation = "oci-native-ingress.oraclecloud.com/healthcheck-protocol" IngressHealthCheckPortAnnotation = "oci-native-ingress.oraclecloud.com/healthcheck-port" @@ -189,6 +191,44 @@ func GetIngressClassDeleteProtectionEnabled(ic *networkingv1.IngressClass) bool return result } +func GetIngressClassDefinedTags(ic *networkingv1.IngressClass) (map[string]map[string]interface{}, error) { + annotation := IngressClassDefinedTagsAnnotation + value, ok := ic.Annotations[annotation] + + // value of defined tags can only be strings for now, but we will allow anything that fits the type + // specified by LoadBalancer.DefinedTags + definedTags := map[string]map[string]interface{}{} + + if !ok || strings.TrimSpace(value) == "" { + return definedTags, nil + } + + err := json.Unmarshal([]byte(value), &definedTags) + if err != nil { + return nil, fmt.Errorf("error parsing value %s for annotation %s: %w", value, annotation, err) + } + + return definedTags, nil +} + +func GetIngressClassFreeformTags(ic *networkingv1.IngressClass) (map[string]string, error) { + annotation := IngressClassFreeformTagsAnnotation + value, ok := ic.Annotations[annotation] + + freeformTags := map[string]string{} + + if !ok || strings.TrimSpace(value) == "" { + return freeformTags, nil + } + + err := json.Unmarshal([]byte(value), &freeformTags) + if err != nil { + return nil, fmt.Errorf("error parsing value %s for annotation %s: %w", value, annotation, err) + } + + return freeformTags, nil +} + func GetIngressProtocol(i *networkingv1.Ingress) string { protocol, ok := i.Annotations[IngressProtocolAnnotation] if !ok { diff --git a/pkg/util/util_test.go b/pkg/util/util_test.go index a3e19011..a53b07e7 100644 --- a/pkg/util/util_test.go +++ b/pkg/util/util_test.go @@ -206,6 +206,82 @@ func TestGetIngressClassDeleteProtectionEnabled(t *testing.T) { Expect(GetIngressClassDeleteProtectionEnabled(ingressClassWithWrongAnnotation)).Should(BeFalse()) } +func TestGetIngressClassDefinedTags(t *testing.T) { + RegisterTestingT(t) + + getIngressClassWithDefinedTagsAnnotation := func(annotation string) *networkingv1.IngressClass { + return &networkingv1.IngressClass{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{IngressClassDefinedTagsAnnotation: annotation}, + }, + } + } + + emptyTags := "" + sampleTags := `{"n1": {"k1": "v1", "k2": {"ik1": "iv1"}},"n2": {"k3": "v3", "k4": ["a1", "a2"]}}` + faultyTags := `{"n1": {"k1"}}` + + emptyTagsResult := map[string]map[string]interface{}{} + sampleTagsResult := map[string]map[string]interface{}{ + "n1": {"k1": "v1", "k2": map[string]interface{}{"ik1": "iv1"}}, + "n2": {"k3": "v3", "k4": []interface{}{"a1", "a2"}}, + } + + ingressClassWithNoAnnotation := &networkingv1.IngressClass{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{}}} + ingressClassWithEmptyTags := getIngressClassWithDefinedTagsAnnotation(emptyTags) + ingressClassWithSampleTags := getIngressClassWithDefinedTagsAnnotation(sampleTags) + ingressClassWithFaultyTags := getIngressClassWithDefinedTagsAnnotation(faultyTags) + + verifyGetIngressClassDefinedTags := func(ic *networkingv1.IngressClass, shouldError bool, + expectedResult map[string]map[string]interface{}) { + res, err := GetIngressClassDefinedTags(ic) + Expect(err != nil).To(Equal(shouldError)) + Expect(res).To(Equal(expectedResult)) + } + + verifyGetIngressClassDefinedTags(ingressClassWithNoAnnotation, false, emptyTagsResult) + verifyGetIngressClassDefinedTags(ingressClassWithEmptyTags, false, emptyTagsResult) + verifyGetIngressClassDefinedTags(ingressClassWithSampleTags, false, sampleTagsResult) + verifyGetIngressClassDefinedTags(ingressClassWithFaultyTags, true, nil) +} + +func TestGetIngressClassFreeformTags(t *testing.T) { + RegisterTestingT(t) + + getIngressClassWithFreeformTagsAnnotation := func(annotation string) *networkingv1.IngressClass { + return &networkingv1.IngressClass{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{IngressClassFreeformTagsAnnotation: annotation}, + }, + } + } + + emptyTags := "" + sampleTags := `{"k1": "v1", "k2": "v2","k3":"v3"}` + faultyTags := `{"k1": v1}` + + emptyTagsResult := map[string]string{} + sampleTagsResult := map[string]string{ + "k1": "v1", "k2": "v2", "k3": "v3", + } + + ingressClassWithNoAnnotation := &networkingv1.IngressClass{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{}}} + ingressClassWithEmptyTags := getIngressClassWithFreeformTagsAnnotation(emptyTags) + ingressClassWithSampleTags := getIngressClassWithFreeformTagsAnnotation(sampleTags) + ingressClassWithFaultyTags := getIngressClassWithFreeformTagsAnnotation(faultyTags) + + verifyGetIngressClassFreeformTags := func(ic *networkingv1.IngressClass, shouldError bool, expectedResult map[string]string) { + res, err := GetIngressClassFreeformTags(ic) + Expect(err != nil).To(Equal(shouldError)) + Expect(res).To(Equal(expectedResult)) + } + + verifyGetIngressClassFreeformTags(ingressClassWithNoAnnotation, false, emptyTagsResult) + verifyGetIngressClassFreeformTags(ingressClassWithEmptyTags, false, emptyTagsResult) + verifyGetIngressClassFreeformTags(ingressClassWithSampleTags, false, sampleTagsResult) + verifyGetIngressClassFreeformTags(ingressClassWithFaultyTags, true, nil) +} + func TestGetIngressClassLoadBalancerId(t *testing.T) { RegisterTestingT(t) lbId := "lbId"