From ae4438bc1151a3c9e3a042b84c35fb8c3d7e917a Mon Sep 17 00:00:00 2001 From: Ashley Dumaine Date: Tue, 5 Mar 2024 17:25:54 -0500 Subject: [PATCH] lint --- cloud/services/firewalls.go | 81 +++++++++++++++++--------- controller/linodecluster_controller.go | 21 ++++--- 2 files changed, 62 insertions(+), 40 deletions(-) diff --git a/cloud/services/firewalls.go b/cloud/services/firewalls.go index d7fc5bdb4..ef30df20b 100644 --- a/cloud/services/firewalls.go +++ b/cloud/services/firewalls.go @@ -5,7 +5,6 @@ import ( "encoding/json" "errors" "fmt" - "github.com/linode/cluster-api-provider-linode/util" "net/http" "slices" @@ -13,6 +12,7 @@ import ( "github.com/linode/linodego" infrav1alpha1 "github.com/linode/cluster-api-provider-linode/api/v1alpha1" + "github.com/linode/cluster-api-provider-linode/util" ) const ( @@ -59,38 +59,18 @@ func HandleFirewall( if len(linodeFWs) == 0 { logger.Info(fmt.Sprintf("Creating firewall %s", firewall.Name)) + linodeFW, err = createFirewall(ctx, linodeClient, fwConfig) + if err != nil { + logger.Info("Failed to create firewall", "error", err.Error()) - if linodeFW, err = linodeClient.CreateFirewall(ctx, *fwConfig); err != nil { - logger.Info("Failed to create Linode Firewall", "error", err.Error()) - // Already exists is not an error - apiErr := linodego.Error{} - if errors.As(err, &apiErr) && apiErr.Code != http.StatusFound { - return nil, err - } - - if linodeFW != nil { - logger.Info(fmt.Sprintf("Linode Firewall %s already exists", firewall.Name)) - } + return nil, err } - } else { logger.Info(fmt.Sprintf("Updating firewall %s", firewall.Name)) linodeFW = &linodeFWs[0] - if !slices.Contains(linodeFW.Tags, clusterUID) { - err := errors.New("firewall conflict") - logger.Error(err, fmt.Sprintf( - "Firewall %s is not associated with cluster UID %s. Owner cluster is %s", - firewall.Name, - clusterUID, - linodeFW.Tags[0], - )) - - return nil, err - } - - if _, err := linodeClient.UpdateFirewallRules(ctx, linodeFW.ID, fwConfig.Rules); err != nil { - logger.Info("Failed to update Linode Firewall", "error", err.Error()) + if err = updateFirewall(ctx, linodeClient, linodeFW, clusterUID, fwConfig); err != nil { + logger.Info("Failed to udpate firewall", "error", err.Error()) return nil, err } @@ -120,6 +100,47 @@ func HandleFirewall( return linodeFW, nil } +func createFirewall( + ctx context.Context, + linodeClient *linodego.Client, + fwConfig *linodego.FirewallCreateOptions, +) (linodeFW *linodego.Firewall, err error) { + if linodeFW, err = linodeClient.CreateFirewall(ctx, *fwConfig); err != nil { + // Already exists is not an error + apiErr := linodego.Error{} + if errors.As(err, &apiErr) && apiErr.Code != http.StatusFound { + return nil, err + } + } + + return linodeFW, nil +} + +func updateFirewall( + ctx context.Context, + linodeClient *linodego.Client, + linodeFW *linodego.Firewall, + clusterUID string, + fwConfig *linodego.FirewallCreateOptions, +) error { + if !slices.Contains(linodeFW.Tags, clusterUID) { + err := fmt.Errorf( + "firewall %s is not associated with cluster UID %s. Owner cluster is %s", + linodeFW.Label, + clusterUID, + linodeFW.Tags[0], + ) + + return err + } + + if _, err := linodeClient.UpdateFirewallRules(ctx, linodeFW.ID, fwConfig.Rules); err != nil { + return err + } + + return nil +} + // fetch Firewalls returns all Linode firewalls with a label matching the CAPL Firewall name func fetchFirewalls(ctx context.Context, name string, linodeClient linodego.Client) (firewalls []linodego.Firewall, err error) { var linodeFWs []linodego.Firewall @@ -132,6 +153,7 @@ func fetchFirewalls(ctx context.Context, name string, linodeClient linodego.Clie if linodeFWs, err = linodeClient.ListFirewalls(ctx, linodego.NewListOptions(1, string(rawFilter))); err != nil { return nil, err } + return linodeFWs, nil } @@ -163,8 +185,7 @@ func chunkIPs(ips []string) [][]string { return chunks } -// processACL builds out a Linode firewall configuration for a given CAPL Firewall object which can then -// be used to create or update a Linode firewall +//nolint:gocyclo,cyclop // As simple as possible. func processACL(firewall *infrav1alpha1.LinodeFirewall, tags []string) (*linodego.FirewallCreateOptions, error) { createOpts := &linodego.FirewallCreateOptions{ Label: firewall.Name, @@ -172,6 +193,7 @@ func processACL(firewall *infrav1alpha1.LinodeFirewall, tags []string) (*linodeg } // process inbound rules + //nolint:dupl // Code duplication is simplicity in this case. for _, rule := range firewall.Spec.InboundRules { var ruleIPv4s []string var ruleIPv6s []string @@ -228,6 +250,7 @@ func processACL(firewall *infrav1alpha1.LinodeFirewall, tags []string) (*linodeg } // process outbound rules + //nolint:dupl // Code duplication is simplicity in this case. for _, rule := range firewall.Spec.OutboundRules { var ruleIPv4s []string var ruleIPv6s []string diff --git a/controller/linodecluster_controller.go b/controller/linodecluster_controller.go index 711d5f7ba..9e9f12182 100644 --- a/controller/linodecluster_controller.go +++ b/controller/linodecluster_controller.go @@ -20,20 +20,15 @@ import ( "context" "errors" "fmt" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "net/http" "time" - apierrors "k8s.io/apimachinery/pkg/api/errors" - utilerrors "k8s.io/apimachinery/pkg/util/errors" - "github.com/go-logr/logr" - "github.com/linode/cluster-api-provider-linode/cloud/scope" - "github.com/linode/cluster-api-provider-linode/cloud/services" - "github.com/linode/cluster-api-provider-linode/util" - "github.com/linode/cluster-api-provider-linode/util/reconciler" "github.com/linode/linodego" corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/client-go/tools/record" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" cerrs "sigs.k8s.io/cluster-api/errors" @@ -41,14 +36,17 @@ import ( "sigs.k8s.io/cluster-api/util/annotations" "sigs.k8s.io/cluster-api/util/conditions" "sigs.k8s.io/cluster-api/util/predicates" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/source" - ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client" - infrav1alpha1 "github.com/linode/cluster-api-provider-linode/api/v1alpha1" + "github.com/linode/cluster-api-provider-linode/cloud/scope" + "github.com/linode/cluster-api-provider-linode/cloud/services" + "github.com/linode/cluster-api-provider-linode/util" + "github.com/linode/cluster-api-provider-linode/util/reconciler" ) // LinodeClusterReconciler reconciles a LinodeCluster object @@ -203,6 +201,7 @@ func createControlPlaneFirewallSpec(clusterScope *scope.ClusterScope) *infrav1al } controlPlaneRules = append(controlPlaneRules, sshRule) } + return &infrav1alpha1.LinodeFirewallSpec{ ClusterUID: string(linodeCluster.UID), FirewallID: linodeCluster.Spec.ControlPlaneFirewall.FirewallID,