From 266cf1514960f8deadb20c8dcb4dd0fffd9f8d54 Mon Sep 17 00:00:00 2001 From: Tarun Chinmai Sekar Date: Thu, 18 Jan 2024 23:33:08 +0000 Subject: [PATCH 01/19] Firewall implementation --- Dockerfile | 6 + Tiltfile | 94 +++++++++++ cloud/linode/annotations.go | 1 + cloud/linode/client.go | 2 + cloud/linode/loadbalancers.go | 161 ++++++++++++++++++- cloud/linode/mock_client_test.go | 51 ++++++ deploy/chart/templates/clusterrole-rbac.yaml | 3 + deploy/chart/values.yaml | 2 +- examples/firewall.yaml | 11 ++ examples/http-nginx.yaml | 3 + 10 files changed, 329 insertions(+), 5 deletions(-) create mode 100644 Tiltfile create mode 100644 examples/firewall.yaml diff --git a/Dockerfile b/Dockerfile index 20942766..29126f68 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,3 +1,9 @@ +# "docker": build in a Docker build container (default) +# "local": copy from the build context. This is useful for tilt's live_update +# feature, allowing hot reload of the linode-ccm binary for fast +# development iteration. See ./Tiltfile +ARG BUILD_SOURCE="docker" + FROM golang:1.21-alpine as builder RUN mkdir -p /linode WORKDIR /linode diff --git a/Tiltfile b/Tiltfile new file mode 100644 index 00000000..355f50d8 --- /dev/null +++ b/Tiltfile @@ -0,0 +1,94 @@ +load("ext://deployment", "deployment_create") +load("ext://k8s_attach", "k8s_attach") +load("ext://restart_process", "docker_build_with_restart") +load("ext://helm_resource", "helm_resource", "helm_repo") + +allow_k8s_contexts(k8s_context()) +# helm_repo("registry-repo", "https://helm.twun.io", labels=["helm-repos"]) + +# # in order to use this registry you must add `127.0.0.1 registry-proxy` in your /etc/hosts +# # and add http://registry-proxy:5000 to the list of insecure registries in docker daemon. +load("ext://k8s_attach", "k8s_attach") + +# # Deploy a docker egistry to the cluster, this registry will be used for hosting images built by tilt. +# helm_resource( +# "docker-registry", +# "registry-repo/docker-registry", +# namespace="registry", +# flags=[ +# "--create-namespace", +# "--version=2.2.2", +# "--set=metrics.enabled=true", +# "--set=storage=filesystem", +# #This [documentation](https://docs.docker.com/registry/configuration/#debug) gives more detail about the debug endpoint and how it can be used in production +# #Disabling this will hide the registry metrics on the 5001 port +# ], +# labels=["registry"], +# resource_deps=["registry-repo"], +# ) + +local_resource( + "registry-probe", + serve_cmd="sleep infinity", + readiness_probe=probe( + period_secs=15, + http_get=http_get_action(host="registry-proxy", port=5000, path="/"), + ), + labels=["registry"], + #resource_deps=["docker-registry"], +) + +k8s_attach( + "registry-port-forward", + "deployment/docker-registry", + namespace="registry", + port_forwards=[5000], + labels=["registry"], +) +default_registry("registry-proxy:5000", host_from_cluster="docker.local") + +labels = ["ccm-linode"] + +local_resource( + "linode-cloud-controller-manager-linux-amd64", + "make build-linux", + # No glob support :( + deps=[ + "cloud/", + "go.mod", + "go.sum", + "main.go", + "Makefile", + "sentry/", + ], + ignore=[ + "**/*.bin", + "**/gomock*", + "cloud/linode/mock_client_test.go", + ], + labels=labels, +) + +entrypoint = "/linode-cloud-controller-manager-linux" +docker_build_with_restart( + "linode/linode-cloud-controller-manager", + ".", + entrypoint=[entrypoint], + ignore=["**/*.go", "go.*", "Makefile"], + live_update=[ + sync( + "dist/linode-cloud-controller-manager-linux-amd64", + entrypoint, + ), + ], + build_args={"BUILD_SOURCE": "local"}, + platform="linux/amd64", +) + +chart_yaml = helm( + "deploy/chart", + name="ccm-linode", + namespace="kube-system", + values="./tilt.values.yaml", +) +k8s_yaml(chart_yaml) diff --git a/cloud/linode/annotations.go b/cloud/linode/annotations.go index a4185e2f..3893e3f4 100644 --- a/cloud/linode/annotations.go +++ b/cloud/linode/annotations.go @@ -27,6 +27,7 @@ const ( annLinodeHostnameOnlyIngress = "service.beta.kubernetes.io/linode-loadbalancer-hostname-only-ingress" annLinodeLoadBalancerTags = "service.beta.kubernetes.io/linode-loadbalancer-tags" annLinodeCloudFirewallID = "service.beta.kubernetes.io/linode-loadbalancer-firewall-id" + annLinodeCloudFirewallCM = "service.beta.kubernetes.io/linode-loadbalancer-firewall-cm" annLinodeNodePrivateIP = "node.k8s.linode.com/private-ip" annLinodeHostUUID = "node.k8s.linode.com/host-uuid" diff --git a/cloud/linode/client.go b/cloud/linode/client.go index 1734366c..a2fcde09 100644 --- a/cloud/linode/client.go +++ b/cloud/linode/client.go @@ -32,6 +32,8 @@ type Client interface { CreateFirewallDevice(ctx context.Context, firewallID int, opts linodego.FirewallDeviceCreateOptions) (*linodego.FirewallDevice, error) CreateFirewall(ctx context.Context, opts linodego.FirewallCreateOptions) (*linodego.Firewall, error) DeleteFirewall(ctx context.Context, fwid int) error + GetFirewall(context.Context, int) (*linodego.Firewall, error) + UpdateFirewallRules(context.Context, int, linodego.FirewallRuleSet) (*linodego.FirewallRuleSet, error) } // linodego.Client implements Client diff --git a/cloud/linode/loadbalancers.go b/cloud/linode/loadbalancers.go index fbbbe6a4..c5316d64 100644 --- a/cloud/linode/loadbalancers.go +++ b/cloud/linode/loadbalancers.go @@ -24,7 +24,16 @@ import ( "github.com/linode/linodego" ) -var errNoNodesAvailable = errors.New("no nodes available for nodebalancer") +var ( + errNoNodesAvailable = errors.New("no nodes available for nodebalancer") + errInvalidFWConfig = errors.New("Specify either an allowList or a denyList for a firewall") + errTooManyFirewalls = errors.New("Too many firewalls attached to a nodebalancer") +) + +const ( + allowListKey = "allowList" + denyListKey = "denyList" +) type lbNotFoundError struct { serviceNn string @@ -39,9 +48,8 @@ func (e lbNotFoundError) Error() string { } type loadbalancers struct { - client Client - zone string - + client Client + zone string kubeClient kubernetes.Interface } @@ -323,6 +331,64 @@ func (l *loadbalancers) updateNodeBalancerFirewall(ctx context.Context, service return nil } +func (l *loadbalancers) reconcileFirewall(ctx context.Context, nb *linodego.NodeBalancer, service *v1.Service) error { + // First things first, see if we need to do anything + // We do that by looking at the fw id annotation, and if it doesn't exist, look at the configMap annotation. + fwid, ok := service.GetAnnotations()[annLinodeCloudFirewallID] + if ok { + _, err := strconv.Atoi(fwid) + if err != nil { + return err + } + // this is a TODO + return nil + } else { + // There's no firewallID already set, see if a configMap exists + fwCM, fwCMannotationExists := service.GetAnnotations()[annLinodeCloudFirewallCM] + // Lots to do here. the user may have changed the configmap's name, we should look at what rules exist on the fw first + // and then reconcile it. + firewalls, err := l.client.ListNodeBalancerFirewalls(ctx, nb.ID, &linodego.ListOptions{}) + if err != nil { + return err + } + + if len(firewalls) > 1 { + // we do not support this case. This means that the customer likely attached a different firewall to this nb, + // don't want to risk changing stuff here. + return errTooManyFirewalls + } + + if !fwCMannotationExists && len(firewalls) == 1 { + // No firewall annotation, but there is a firewall attached our node-balancer. we should remove it. + err := l.client.DeleteFirewallDevice(ctx, firewalls[0].ID, nb.ID) + if err != nil { + return err + } + // once we delete the device, we should see if there's anything attached to that firewall + devices, err := l.client.ListFirewallDevices(ctx, firewalls[0].ID, &linodego.ListOptions{}) + if err != nil { + return err + } + if len(devices) == 0 { + // nothing attached to it, clean it up + return l.client.DeleteFirewall(ctx, firewalls[0].ID) + } + // else let that firewall linger, don't mess with it. + } + + // We do not want to get into the complexity of reconciling differences, might as well just pull what's in the configMap now and update the fw. + fwCreateOpts, err := l.createFirewallOptsForSvc(ctx, fwCM, "", []string{""}, service.Spec.Ports) + if err != nil { + return err + } + _, err = l.client.UpdateFirewallRules(ctx, firewalls[0].ID, fwCreateOpts.Rules) + if err != nil { + return err + } + } + return nil +} + //nolint:funlen func (l *loadbalancers) updateNodeBalancer(ctx context.Context, clusterName string, service *v1.Service, nodes []*v1.Node, nb *linodego.NodeBalancer) (err error) { if len(nodes) == 0 { @@ -428,6 +494,14 @@ func (l *loadbalancers) updateNodeBalancer(ctx context.Context, clusterName stri return fmt.Errorf("[port %d] error rebuilding NodeBalancer config: %v", int(port.Port), err) } } + + // Firewall configs + // Delete any configs for ports that have been removed from the Service + if err = l.reconcileFirewall(ctx, nb, service); err != nil { + sentry.CaptureError(ctx, err) + return err + } + return nil } @@ -580,6 +654,73 @@ func (l *loadbalancers) getLoadBalancerTags(_ context.Context, clusterName strin return tags } +func processACL(fwcreateOpts *linodego.FirewallCreateOptions, acl string, aclType, label, cmName, ports string) error { + var allowedIPs linodego.NetworkAddresses + err := json.Unmarshal([]byte(acl), &allowedIPs) + if err != nil { + klog.Error("Error marshalling IPs in allowList") + return err + } + + fwcreateOpts.Rules.Inbound = append(fwcreateOpts.Rules.Inbound, linodego.FirewallRule{ + Action: aclType, + Label: fmt.Sprintf("%s-%s", aclType, cmName), + Description: fmt.Sprintf("Created by linode-ccm: %s, from %s", label, cmName), + Protocol: linodego.TCP, // Nodebalancers support only TCP. + Ports: ports, + Addresses: allowedIPs, + }) + fwcreateOpts.Rules.OutboundPolicy = "ACCEPT" + if aclType == "ACCEPT" { + // if an allowlist is present, we drop everything else. + fwcreateOpts.Rules.InboundPolicy = "DROP" + } else { + // if a denylist is present, we accept everything else. + fwcreateOpts.Rules.InboundPolicy = "ACCEPT" + } + return nil +} + +func (l *loadbalancers) createFirewallOptsForSvc(ctx context.Context, fwCM, label string, tags []string, ports []v1.ServicePort) (*linodego.FirewallCreateOptions, error) { + // Fetch config map + cm, err := l.kubeClient.CoreV1().ConfigMaps("kube-system").Get(ctx, fwCM, metav1.GetOptions{}) + if err != nil { + return nil, err + } + fwcreateOpts := linodego.FirewallCreateOptions{ + Label: label, + Tags: tags, + } + + servicePorts := make([]string, len(ports)) + for idx, port := range ports { + servicePorts[idx] = strconv.Itoa(int(port.Port)) + } + + portsString := strings.Join(servicePorts[:], ",") + + allowListJson, allowListOk := cm.Data[allowListKey] + denyListJson, denyListOk := cm.Data[denyListKey] + + if (allowListOk && denyListOk) || (!allowListOk && !denyListOk) { + // it is a problem if both are set, or if both are not set + return nil, errInvalidFWConfig + } + + aclType := "ACCEPT" + acl := allowListJson + if denyListOk { + aclType = "DROP" + acl = denyListJson + } + + err = processACL(&fwcreateOpts, acl, aclType, label, cm.Name, portsString) + if err != nil { + return nil, err + } + return &fwcreateOpts, err +} + func (l *loadbalancers) createNodeBalancer(ctx context.Context, clusterName string, service *v1.Service, configs []*linodego.NodeBalancerConfigCreateOptions) (lb *linodego.NodeBalancer, err error) { connThrottle := getConnectionThrottle(service) @@ -600,6 +741,18 @@ func (l *loadbalancers) createNodeBalancer(ctx context.Context, clusterName stri return nil, err } createOpts.FirewallID = firewallID + } else { + // There's no firewallID already set, see if we need to create a new fw + fwCM, ok := getServiceAnnotation(service, annLinodeCloudFirewallCM) + if ok { + fwcreateOpts, err := l.createFirewallOptsForSvc(ctx, fwCM, label, tags, service.Spec.Ports) + firewall, err := l.client.CreateFirewall(ctx, *fwcreateOpts) + if err != nil { + return nil, err + } + createOpts.FirewallID = firewall.ID + } + // no need to deal with firewalls, continue creating nb's } return l.client.CreateNodeBalancer(ctx, createOpts) diff --git a/cloud/linode/mock_client_test.go b/cloud/linode/mock_client_test.go index 073ccd4c..2c1aca17 100644 --- a/cloud/linode/mock_client_test.go +++ b/cloud/linode/mock_client_test.go @@ -50,6 +50,7 @@ func (mr *MockClientMockRecorder) CreateFirewall(arg0, arg1 interface{}) *gomock return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateFirewall", reflect.TypeOf((*MockClient)(nil).CreateFirewall), arg0, arg1) } +<<<<<<< HEAD // CreateFirewallDevice mocks base method. func (m *MockClient) CreateFirewallDevice(arg0 context.Context, arg1 int, arg2 linodego.FirewallDeviceCreateOptions) (*linodego.FirewallDevice, error) { m.ctrl.T.Helper() @@ -65,6 +66,8 @@ func (mr *MockClientMockRecorder) CreateFirewallDevice(arg0, arg1, arg2 interfac return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateFirewallDevice", reflect.TypeOf((*MockClient)(nil).CreateFirewallDevice), arg0, arg1, arg2) } +======= +>>>>>>> d903958 (Firewall implementation) // CreateNodeBalancer mocks base method. func (m *MockClient) CreateNodeBalancer(arg0 context.Context, arg1 linodego.NodeBalancerCreateOptions) (*linodego.NodeBalancer, error) { m.ctrl.T.Helper() @@ -109,6 +112,7 @@ func (mr *MockClientMockRecorder) DeleteFirewall(arg0, arg1 interface{}) *gomock return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteFirewall", reflect.TypeOf((*MockClient)(nil).DeleteFirewall), arg0, arg1) } +<<<<<<< HEAD // DeleteFirewallDevice mocks base method. func (m *MockClient) DeleteFirewallDevice(arg0 context.Context, arg1, arg2 int) error { m.ctrl.T.Helper() @@ -123,6 +127,8 @@ func (mr *MockClientMockRecorder) DeleteFirewallDevice(arg0, arg1, arg2 interfac return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteFirewallDevice", reflect.TypeOf((*MockClient)(nil).DeleteFirewallDevice), arg0, arg1, arg2) } +======= +>>>>>>> d903958 (Firewall implementation) // DeleteNodeBalancer mocks base method. func (m *MockClient) DeleteNodeBalancer(arg0 context.Context, arg1 int) error { m.ctrl.T.Helper() @@ -151,6 +157,21 @@ func (mr *MockClientMockRecorder) DeleteNodeBalancerConfig(arg0, arg1, arg2 inte return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteNodeBalancerConfig", reflect.TypeOf((*MockClient)(nil).DeleteNodeBalancerConfig), arg0, arg1, arg2) } +// GetFirewall mocks base method. +func (m *MockClient) GetFirewall(arg0 context.Context, arg1 int) (*linodego.Firewall, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetFirewall", arg0, arg1) + ret0, _ := ret[0].(*linodego.Firewall) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetFirewall indicates an expected call of GetFirewall. +func (mr *MockClientMockRecorder) GetFirewall(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetFirewall", reflect.TypeOf((*MockClient)(nil).GetFirewall), arg0, arg1) +} + // GetInstance mocks base method. func (m *MockClient) GetInstance(arg0 context.Context, arg1 int) (*linodego.Instance, error) { m.ctrl.T.Helper() @@ -286,6 +307,36 @@ func (mr *MockClientMockRecorder) RebuildNodeBalancerConfig(arg0, arg1, arg2, ar return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RebuildNodeBalancerConfig", reflect.TypeOf((*MockClient)(nil).RebuildNodeBalancerConfig), arg0, arg1, arg2, arg3) } +// UpdateFirewall mocks base method. +func (m *MockClient) UpdateFirewall(arg0 context.Context, arg1 int, arg2 linodego.FirewallUpdateOptions) (*linodego.Firewall, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UpdateFirewall", arg0, arg1, arg2) + ret0, _ := ret[0].(*linodego.Firewall) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// UpdateFirewall indicates an expected call of UpdateFirewall. +func (mr *MockClientMockRecorder) UpdateFirewall(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateFirewall", reflect.TypeOf((*MockClient)(nil).UpdateFirewall), arg0, arg1, arg2) +} + +// UpdateFirewallRules mocks base method. +func (m *MockClient) UpdateFirewallRules(arg0 context.Context, arg1 int, arg2 linodego.FirewallRuleSet) (*linodego.FirewallRuleSet, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UpdateFirewallRules", arg0, arg1, arg2) + ret0, _ := ret[0].(*linodego.FirewallRuleSet) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// UpdateFirewallRules indicates an expected call of UpdateFirewallRules. +func (mr *MockClientMockRecorder) UpdateFirewallRules(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateFirewallRules", reflect.TypeOf((*MockClient)(nil).UpdateFirewallRules), arg0, arg1, arg2) +} + // UpdateNodeBalancer mocks base method. func (m *MockClient) UpdateNodeBalancer(arg0 context.Context, arg1 int, arg2 linodego.NodeBalancerUpdateOptions) (*linodego.NodeBalancer, error) { m.ctrl.T.Helper() diff --git a/deploy/chart/templates/clusterrole-rbac.yaml b/deploy/chart/templates/clusterrole-rbac.yaml index 29532135..588f51ec 100644 --- a/deploy/chart/templates/clusterrole-rbac.yaml +++ b/deploy/chart/templates/clusterrole-rbac.yaml @@ -21,6 +21,9 @@ rules: - apiGroups: [""] resources: ["secrets"] verbs: ["get"] + - apiGroups: [""] + resources: ["configmaps"] + verbs: ["get"] - apiGroups: [""] resources: ["services"] verbs: ["get", "watch", "list"] diff --git a/deploy/chart/values.yaml b/deploy/chart/values.yaml index 790458df..7b2e3804 100644 --- a/deploy/chart/values.yaml +++ b/deploy/chart/values.yaml @@ -48,4 +48,4 @@ tolerations: # LINODE_HOSTNAME_ONLY_INGRESS type bool is supported # env: # - name: EXAMPLE_ENV_VAR - # value: "true" \ No newline at end of file + # value: "true" diff --git a/examples/firewall.yaml b/examples/firewall.yaml new file mode 100644 index 00000000..78a3525a --- /dev/null +++ b/examples/firewall.yaml @@ -0,0 +1,11 @@ +--- +kind: ConfigMap +apiVersion: v1 +metadata: + name: "firewall-rules" + namespace: "kube-system" +data: + allowList: | + { + "ipv4": ["8.8.8.8/32"] + } diff --git a/examples/http-nginx.yaml b/examples/http-nginx.yaml index 43312bb9..a2dfc508 100644 --- a/examples/http-nginx.yaml +++ b/examples/http-nginx.yaml @@ -3,8 +3,10 @@ kind: Service apiVersion: v1 metadata: name: http-lb + namespace: kube-system annotations: service.beta.kubernetes.io/linode-loadbalancer-default-protocol: "http" + service.beta.kubernetes.io/linode-loadbalancer-firewall-cm: "firewall-rules" spec: type: LoadBalancer selector: @@ -20,6 +22,7 @@ apiVersion: apps/v1 kind: Deployment metadata: name: nginx-http-deployment + namespace: kube-system spec: replicas: 2 selector: From 810e9afcc41eb649b6bb767772fa477de3d65aa1 Mon Sep 17 00:00:00 2001 From: Tarun Chinmai Sekar Date: Mon, 22 Jan 2024 19:06:51 +0000 Subject: [PATCH 02/19] specify acl on service directly --- cloud/linode/annotations.go | 2 +- cloud/linode/loadbalancers.go | 57 +++++++++++++++++------------------ examples/firewall.yaml | 11 ------- examples/http-nginx.yaml | 9 ++++-- 4 files changed, 35 insertions(+), 44 deletions(-) delete mode 100644 examples/firewall.yaml diff --git a/cloud/linode/annotations.go b/cloud/linode/annotations.go index 3893e3f4..18c0f874 100644 --- a/cloud/linode/annotations.go +++ b/cloud/linode/annotations.go @@ -27,7 +27,7 @@ const ( annLinodeHostnameOnlyIngress = "service.beta.kubernetes.io/linode-loadbalancer-hostname-only-ingress" annLinodeLoadBalancerTags = "service.beta.kubernetes.io/linode-loadbalancer-tags" annLinodeCloudFirewallID = "service.beta.kubernetes.io/linode-loadbalancer-firewall-id" - annLinodeCloudFirewallCM = "service.beta.kubernetes.io/linode-loadbalancer-firewall-cm" + annLinodeCloudFirewallACL = "service.beta.kubernetes.io/linode-loadbalancer-firewall-acl" annLinodeNodePrivateIP = "node.k8s.linode.com/private-ip" annLinodeHostUUID = "node.k8s.linode.com/host-uuid" diff --git a/cloud/linode/loadbalancers.go b/cloud/linode/loadbalancers.go index c5316d64..29f1b46a 100644 --- a/cloud/linode/loadbalancers.go +++ b/cloud/linode/loadbalancers.go @@ -654,21 +654,14 @@ func (l *loadbalancers) getLoadBalancerTags(_ context.Context, clusterName strin return tags } -func processACL(fwcreateOpts *linodego.FirewallCreateOptions, acl string, aclType, label, cmName, ports string) error { - var allowedIPs linodego.NetworkAddresses - err := json.Unmarshal([]byte(acl), &allowedIPs) - if err != nil { - klog.Error("Error marshalling IPs in allowList") - return err - } - +func processACL(fwcreateOpts *linodego.FirewallCreateOptions, aclType, label, svcName, ports string, ips linodego.NetworkAddresses) error { fwcreateOpts.Rules.Inbound = append(fwcreateOpts.Rules.Inbound, linodego.FirewallRule{ Action: aclType, - Label: fmt.Sprintf("%s-%s", aclType, cmName), - Description: fmt.Sprintf("Created by linode-ccm: %s, from %s", label, cmName), + Label: fmt.Sprintf("%s-%s", aclType, svcName), + Description: fmt.Sprintf("Created by linode-ccm: %s, for %s", label, svcName), Protocol: linodego.TCP, // Nodebalancers support only TCP. Ports: ports, - Addresses: allowedIPs, + Addresses: ips, }) fwcreateOpts.Rules.OutboundPolicy = "ACCEPT" if aclType == "ACCEPT" { @@ -681,40 +674,44 @@ func processACL(fwcreateOpts *linodego.FirewallCreateOptions, acl string, aclTyp return nil } -func (l *loadbalancers) createFirewallOptsForSvc(ctx context.Context, fwCM, label string, tags []string, ports []v1.ServicePort) (*linodego.FirewallCreateOptions, error) { - // Fetch config map - cm, err := l.kubeClient.CoreV1().ConfigMaps("kube-system").Get(ctx, fwCM, metav1.GetOptions{}) - if err != nil { - return nil, err - } +type aclConfig struct { + AllowList *linodego.NetworkAddresses `json:"allowList"` + DenyList *linodego.NetworkAddresses `json:"denyList"` +} + +func (l *loadbalancers) createFirewallOptsForSvc(ctx context.Context, label string, tags []string, svc *v1.Service) (*linodego.FirewallCreateOptions, error) { + // Fetch acl from annotation + aclString := svc.GetAnnotations()[annLinodeCloudFirewallACL] fwcreateOpts := linodego.FirewallCreateOptions{ Label: label, Tags: tags, } - servicePorts := make([]string, len(ports)) - for idx, port := range ports { + servicePorts := make([]string, len(svc.Spec.Ports)) + for idx, port := range svc.Spec.Ports { servicePorts[idx] = strconv.Itoa(int(port.Port)) } portsString := strings.Join(servicePorts[:], ",") + var acl aclConfig + err := json.Unmarshal([]byte(aclString), &acl) + if err != nil { + return nil, err + } - allowListJson, allowListOk := cm.Data[allowListKey] - denyListJson, denyListOk := cm.Data[denyListKey] - - if (allowListOk && denyListOk) || (!allowListOk && !denyListOk) { + if (acl.AllowList != nil && acl.DenyList != nil) || (acl.AllowList == nil && acl.DenyList == nil) { // it is a problem if both are set, or if both are not set return nil, errInvalidFWConfig } aclType := "ACCEPT" - acl := allowListJson - if denyListOk { + allowedIPs := acl.AllowList + if acl.DenyList != nil { aclType = "DROP" - acl = denyListJson + allowedIPs = acl.DenyList } - err = processACL(&fwcreateOpts, acl, aclType, label, cm.Name, portsString) + err = processACL(&fwcreateOpts, aclType, label, svc.Name, portsString, *allowedIPs) if err != nil { return nil, err } @@ -742,10 +739,10 @@ func (l *loadbalancers) createNodeBalancer(ctx context.Context, clusterName stri } createOpts.FirewallID = firewallID } else { - // There's no firewallID already set, see if we need to create a new fw - fwCM, ok := getServiceAnnotation(service, annLinodeCloudFirewallCM) + // There's no firewallID already set, see if we need to create a new fw, look for the acl annotation. + _, ok := service.GetAnnotations()[annLinodeCloudFirewallACL] if ok { - fwcreateOpts, err := l.createFirewallOptsForSvc(ctx, fwCM, label, tags, service.Spec.Ports) + fwcreateOpts, err := l.createFirewallOptsForSvc(ctx, label, tags, service) firewall, err := l.client.CreateFirewall(ctx, *fwcreateOpts) if err != nil { return nil, err diff --git a/examples/firewall.yaml b/examples/firewall.yaml deleted file mode 100644 index 78a3525a..00000000 --- a/examples/firewall.yaml +++ /dev/null @@ -1,11 +0,0 @@ ---- -kind: ConfigMap -apiVersion: v1 -metadata: - name: "firewall-rules" - namespace: "kube-system" -data: - allowList: | - { - "ipv4": ["8.8.8.8/32"] - } diff --git a/examples/http-nginx.yaml b/examples/http-nginx.yaml index a2dfc508..b712da38 100644 --- a/examples/http-nginx.yaml +++ b/examples/http-nginx.yaml @@ -6,7 +6,12 @@ metadata: namespace: kube-system annotations: service.beta.kubernetes.io/linode-loadbalancer-default-protocol: "http" - service.beta.kubernetes.io/linode-loadbalancer-firewall-cm: "firewall-rules" + service.beta.kubernetes.io/linode-loadbalancer-firewall-acl: | + { + "denyList": { + "ipv4": ["8.8.8.8/32"], + } + } spec: type: LoadBalancer selector: @@ -22,7 +27,7 @@ apiVersion: apps/v1 kind: Deployment metadata: name: nginx-http-deployment - namespace: kube-system + namespace: kube-system spec: replicas: 2 selector: From 1e4a3897abcdfcaae860ab2c2bb3189fdd39d106 Mon Sep 17 00:00:00 2001 From: Tarun Chinmai Sekar Date: Mon, 22 Jan 2024 22:32:37 +0000 Subject: [PATCH 03/19] fixups + test code from other branch --- cloud/linode/client.go | 7 +++ cloud/linode/loadbalancers.go | 18 +++--- cloud/linode/loadbalancers_test.go | 91 +++++++++++++++++++++++++++--- cloud/linode/mock_client_test.go | 21 ------- examples/http-nginx.yaml | 3 +- 5 files changed, 102 insertions(+), 38 deletions(-) diff --git a/cloud/linode/client.go b/cloud/linode/client.go index a2fcde09..4b839419 100644 --- a/cloud/linode/client.go +++ b/cloud/linode/client.go @@ -34,6 +34,13 @@ type Client interface { DeleteFirewall(ctx context.Context, fwid int) error GetFirewall(context.Context, int) (*linodego.Firewall, error) UpdateFirewallRules(context.Context, int, linodego.FirewallRuleSet) (*linodego.FirewallRuleSet, error) +<<<<<<< HEAD +======= + DeleteFirewall(context.Context, int) error + ListNodeBalancerFirewalls(ctx context.Context, nodebalancerID int, opts *linodego.ListOptions) ([]linodego.Firewall, error) + DeleteFirewallDevice(ctx context.Context, firewallID, deviceID int) error + ListFirewallDevices(ctx context.Context, firewallID int, opts *linodego.ListOptions) ([]linodego.FirewallDevice, error) +>>>>>>> dadcd59 (fixups + test code from other branch) } // linodego.Client implements Client diff --git a/cloud/linode/loadbalancers.go b/cloud/linode/loadbalancers.go index 29f1b46a..6359cdb5 100644 --- a/cloud/linode/loadbalancers.go +++ b/cloud/linode/loadbalancers.go @@ -343,9 +343,9 @@ func (l *loadbalancers) reconcileFirewall(ctx context.Context, nb *linodego.Node // this is a TODO return nil } else { - // There's no firewallID already set, see if a configMap exists - fwCM, fwCMannotationExists := service.GetAnnotations()[annLinodeCloudFirewallCM] - // Lots to do here. the user may have changed the configmap's name, we should look at what rules exist on the fw first + // There's no firewallID already set, see if a acl exists + _, fwACLExists := service.GetAnnotations()[annLinodeCloudFirewallACL] + // Lots to do here. the user may have changed the acl, we should look at what rules exist on the fw first // and then reconcile it. firewalls, err := l.client.ListNodeBalancerFirewalls(ctx, nb.ID, &linodego.ListOptions{}) if err != nil { @@ -358,7 +358,7 @@ func (l *loadbalancers) reconcileFirewall(ctx context.Context, nb *linodego.Node return errTooManyFirewalls } - if !fwCMannotationExists && len(firewalls) == 1 { + if !fwACLExists && len(firewalls) == 1 { // No firewall annotation, but there is a firewall attached our node-balancer. we should remove it. err := l.client.DeleteFirewallDevice(ctx, firewalls[0].ID, nb.ID) if err != nil { @@ -377,7 +377,7 @@ func (l *loadbalancers) reconcileFirewall(ctx context.Context, nb *linodego.Node } // We do not want to get into the complexity of reconciling differences, might as well just pull what's in the configMap now and update the fw. - fwCreateOpts, err := l.createFirewallOptsForSvc(ctx, fwCM, "", []string{""}, service.Spec.Ports) + fwCreateOpts, err := l.createFirewallOptsForSvc(ctx, service.Name, []string{""}, service) if err != nil { return err } @@ -698,9 +698,8 @@ func (l *loadbalancers) createFirewallOptsForSvc(ctx context.Context, label stri if err != nil { return nil, err } - + // it is a problem if both are set, or if both are not set if (acl.AllowList != nil && acl.DenyList != nil) || (acl.AllowList == nil && acl.DenyList == nil) { - // it is a problem if both are set, or if both are not set return nil, errInvalidFWConfig } @@ -743,8 +742,13 @@ func (l *loadbalancers) createNodeBalancer(ctx context.Context, clusterName stri _, ok := service.GetAnnotations()[annLinodeCloudFirewallACL] if ok { fwcreateOpts, err := l.createFirewallOptsForSvc(ctx, label, tags, service) + if err != nil { + return nil, err + } + firewall, err := l.client.CreateFirewall(ctx, *fwcreateOpts) if err != nil { + fmt.Printf("TARUN here %v", err) return nil, err } createOpts.FirewallID = firewall.ID diff --git a/cloud/linode/loadbalancers_test.go b/cloud/linode/loadbalancers_test.go index 311e6f15..ac6850de 100644 --- a/cloud/linode/loadbalancers_test.go +++ b/cloud/linode/loadbalancers_test.go @@ -125,6 +125,18 @@ func TestCCMLoadBalancers(t *testing.T) { { name: "Create Load Balancer With Invalid Firewall ID", f: testCreateNodeBalancerWithInvalidFirewall, + }, { + name: "Create Load Balancer With Valid Firewall ACL - AllowList", + f: testCreateNodeBalancerWithAllowList, + }, { + name: "Create Load Balancer With Valid Firewall ACL - DenyList", + f: testCreateNodeBalancerWithDenyList, + }, { + name: "Create Load Balancer With Invalid Firewall ACL - Both Allow and Deny", + f: testCreateNodeBalanceWithBothAllowOrDenyList, + }, { + name: "Create Load Balancer With Invalid Firewall ACL - NO Allow Or Deny", + f: testCreateNodeBalanceWithNoAllowOrDenyList, }, { name: "Update Load Balancer - Add Annotation", @@ -226,7 +238,7 @@ func stubService(fake *fake.Clientset, service *v1.Service) { _, _ = fake.CoreV1().Services("").Create(context.TODO(), service, metav1.CreateOptions{}) } -func testCreateNodeBalancer(t *testing.T, client *linodego.Client, _ *fakeAPI, firewallID *string) error { +func testCreateNodeBalancer(t *testing.T, client *linodego.Client, _ *fakeAPI, annotations map[string]string) error { svc := &v1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: randString(), @@ -253,11 +265,9 @@ func testCreateNodeBalancer(t *testing.T, client *linodego.Client, _ *fakeAPI, f }, }, } - - if firewallID != nil { - svc.Annotations[annLinodeCloudFirewallID] = *firewallID + for key, value := range annotations { + svc.Annotations[key] = value } - lb := &loadbalancers{client, "us-west", nil} nodes := []*v1.Node{ {ObjectMeta: metav1.ObjectMeta{Name: "node-1"}}, @@ -313,18 +323,81 @@ func testCreateNodeBalancerWithOutFirewall(t *testing.T, client *linodego.Client } } +func testCreateNodeBalanceWithNoAllowOrDenyList(t *testing.T, client *linodego.Client, f *fakeAPI) { + annotations := map[string]string{ + annLinodeCloudFirewallACL: `{}`, + } + + err := testCreateNodeBalancer(t, client, f, annotations) + if err == nil || !stderrors.Is(err, errInvalidFWConfig) { + t.Fatalf("expected a %v error, got %v", errInvalidFWConfig, err) + } +} + +func testCreateNodeBalanceWithBothAllowOrDenyList(t *testing.T, client *linodego.Client, f *fakeAPI) { + annotations := map[string]string{ + annLinodeCloudFirewallACL: `{ + "allowList": { + "ipv4": ["2.2.2.2"] + }, + "denyList": { + "ipv4": ["2.2.2.2"] + } + }`, + } + + err := testCreateNodeBalancer(t, client, f, annotations) + if err == nil || !stderrors.Is(err, errInvalidFWConfig) { + t.Fatalf("expected a %v error, got %v", errInvalidFWConfig, err) + } +} + +func testCreateNodeBalancerWithAllowList(t *testing.T, client *linodego.Client, f *fakeAPI) { + annotations := map[string]string{ + annLinodeCloudFirewallACL: `{ + "allowList": { + "ipv4": ["2.2.2.2"] + } + }`, + } + + err := testCreateNodeBalancer(t, client, f, annotations) + if err == nil { + t.Fatalf("expected a non-nil error, got %v", err) + } +} + +func testCreateNodeBalancerWithDenyList(t *testing.T, client *linodego.Client, f *fakeAPI) { + annotations := map[string]string{ + annLinodeCloudFirewallACL: `{ + "denyList": { + "ipv4": ["2.2.2.2"] + } + }`, + } + + err := testCreateNodeBalancer(t, client, f, annotations) + if err != nil { + t.Fatalf("expected a non-nil error, got %v", err) + } +} + func testCreateNodeBalancerWithFirewall(t *testing.T, client *linodego.Client, f *fakeAPI) { - firewallID := "123" - err := testCreateNodeBalancer(t, client, f, &firewallID) + annotations := map[string]string{ + annLinodeCloudFirewallID: "123", + } + err := testCreateNodeBalancer(t, client, f, annotations) if err != nil { t.Fatalf("expected a nil error, got %v", err) } } func testCreateNodeBalancerWithInvalidFirewall(t *testing.T, client *linodego.Client, f *fakeAPI) { - firewallID := "qwerty" + annotations := map[string]string{ + annLinodeCloudFirewallID: "qwerty", + } expectedError := "strconv.Atoi: parsing \"qwerty\": invalid syntax" - err := testCreateNodeBalancer(t, client, f, &firewallID) + err := testCreateNodeBalancer(t, client, f, annotations) if err.Error() != expectedError { t.Fatalf("expected a %s error, got %v", expectedError, err) } diff --git a/cloud/linode/mock_client_test.go b/cloud/linode/mock_client_test.go index 2c1aca17..d7f5b984 100644 --- a/cloud/linode/mock_client_test.go +++ b/cloud/linode/mock_client_test.go @@ -50,7 +50,6 @@ func (mr *MockClientMockRecorder) CreateFirewall(arg0, arg1 interface{}) *gomock return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateFirewall", reflect.TypeOf((*MockClient)(nil).CreateFirewall), arg0, arg1) } -<<<<<<< HEAD // CreateFirewallDevice mocks base method. func (m *MockClient) CreateFirewallDevice(arg0 context.Context, arg1 int, arg2 linodego.FirewallDeviceCreateOptions) (*linodego.FirewallDevice, error) { m.ctrl.T.Helper() @@ -66,8 +65,6 @@ func (mr *MockClientMockRecorder) CreateFirewallDevice(arg0, arg1, arg2 interfac return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateFirewallDevice", reflect.TypeOf((*MockClient)(nil).CreateFirewallDevice), arg0, arg1, arg2) } -======= ->>>>>>> d903958 (Firewall implementation) // CreateNodeBalancer mocks base method. func (m *MockClient) CreateNodeBalancer(arg0 context.Context, arg1 linodego.NodeBalancerCreateOptions) (*linodego.NodeBalancer, error) { m.ctrl.T.Helper() @@ -112,7 +109,6 @@ func (mr *MockClientMockRecorder) DeleteFirewall(arg0, arg1 interface{}) *gomock return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteFirewall", reflect.TypeOf((*MockClient)(nil).DeleteFirewall), arg0, arg1) } -<<<<<<< HEAD // DeleteFirewallDevice mocks base method. func (m *MockClient) DeleteFirewallDevice(arg0 context.Context, arg1, arg2 int) error { m.ctrl.T.Helper() @@ -127,8 +123,6 @@ func (mr *MockClientMockRecorder) DeleteFirewallDevice(arg0, arg1, arg2 interfac return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteFirewallDevice", reflect.TypeOf((*MockClient)(nil).DeleteFirewallDevice), arg0, arg1, arg2) } -======= ->>>>>>> d903958 (Firewall implementation) // DeleteNodeBalancer mocks base method. func (m *MockClient) DeleteNodeBalancer(arg0 context.Context, arg1 int) error { m.ctrl.T.Helper() @@ -307,21 +301,6 @@ func (mr *MockClientMockRecorder) RebuildNodeBalancerConfig(arg0, arg1, arg2, ar return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RebuildNodeBalancerConfig", reflect.TypeOf((*MockClient)(nil).RebuildNodeBalancerConfig), arg0, arg1, arg2, arg3) } -// UpdateFirewall mocks base method. -func (m *MockClient) UpdateFirewall(arg0 context.Context, arg1 int, arg2 linodego.FirewallUpdateOptions) (*linodego.Firewall, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "UpdateFirewall", arg0, arg1, arg2) - ret0, _ := ret[0].(*linodego.Firewall) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// UpdateFirewall indicates an expected call of UpdateFirewall. -func (mr *MockClientMockRecorder) UpdateFirewall(arg0, arg1, arg2 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateFirewall", reflect.TypeOf((*MockClient)(nil).UpdateFirewall), arg0, arg1, arg2) -} - // UpdateFirewallRules mocks base method. func (m *MockClient) UpdateFirewallRules(arg0 context.Context, arg1 int, arg2 linodego.FirewallRuleSet) (*linodego.FirewallRuleSet, error) { m.ctrl.T.Helper() diff --git a/examples/http-nginx.yaml b/examples/http-nginx.yaml index b712da38..0bf70efd 100644 --- a/examples/http-nginx.yaml +++ b/examples/http-nginx.yaml @@ -8,8 +8,9 @@ metadata: service.beta.kubernetes.io/linode-loadbalancer-default-protocol: "http" service.beta.kubernetes.io/linode-loadbalancer-firewall-acl: | { - "denyList": { + "allowList": { "ipv4": ["8.8.8.8/32"], + "ipv6": ["dead:beef::/64"] } } spec: From c8441881b41af34c067e2c361445c24b494d9ad8 Mon Sep 17 00:00:00 2001 From: Tarun Chinmai Sekar Date: Tue, 23 Jan 2024 02:46:59 +0000 Subject: [PATCH 04/19] add tests --- cloud/linode/loadbalancers_test.go | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/cloud/linode/loadbalancers_test.go b/cloud/linode/loadbalancers_test.go index ac6850de..268e393a 100644 --- a/cloud/linode/loadbalancers_test.go +++ b/cloud/linode/loadbalancers_test.go @@ -125,10 +125,12 @@ func TestCCMLoadBalancers(t *testing.T) { { name: "Create Load Balancer With Invalid Firewall ID", f: testCreateNodeBalancerWithInvalidFirewall, - }, { + }, + { name: "Create Load Balancer With Valid Firewall ACL - AllowList", f: testCreateNodeBalancerWithAllowList, - }, { + }, + { name: "Create Load Balancer With Valid Firewall ACL - DenyList", f: testCreateNodeBalancerWithDenyList, }, { @@ -312,6 +314,19 @@ func testCreateNodeBalancer(t *testing.T, client *linodego.Client, _ *fakeAPI, a t.Logf("actual: %v", nb.Tags) } + _, ok := annotations[annLinodeCloudFirewallACL] + if ok { + // a firewall was configured for this + firewalls, err := client.ListNodeBalancerFirewalls(context.TODO(), nb.ID, &linodego.ListOptions{}) + if err != nil { + t.Errorf("Expected nil error, got %v", err) + } + + if len(firewalls) == 0 { + t.Errorf("Expected 1 firewall, got %d", len(firewalls)) + } + } + defer func() { _ = lb.EnsureLoadBalancerDeleted(context.TODO(), "linodelb", svc) }() return nil } @@ -362,9 +377,10 @@ func testCreateNodeBalancerWithAllowList(t *testing.T, client *linodego.Client, } err := testCreateNodeBalancer(t, client, f, annotations) - if err == nil { + if err != nil { t.Fatalf("expected a non-nil error, got %v", err) } + } func testCreateNodeBalancerWithDenyList(t *testing.T, client *linodego.Client, f *fakeAPI) { From 2c7da29a4e0693e103097b7759f6dc7a747ada7f Mon Sep 17 00:00:00 2001 From: Tarun Chinmai Sekar Date: Tue, 23 Jan 2024 21:23:54 +0000 Subject: [PATCH 05/19] rebase --- cloud/linode/client.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/cloud/linode/client.go b/cloud/linode/client.go index 4b839419..a2fcde09 100644 --- a/cloud/linode/client.go +++ b/cloud/linode/client.go @@ -34,13 +34,6 @@ type Client interface { DeleteFirewall(ctx context.Context, fwid int) error GetFirewall(context.Context, int) (*linodego.Firewall, error) UpdateFirewallRules(context.Context, int, linodego.FirewallRuleSet) (*linodego.FirewallRuleSet, error) -<<<<<<< HEAD -======= - DeleteFirewall(context.Context, int) error - ListNodeBalancerFirewalls(ctx context.Context, nodebalancerID int, opts *linodego.ListOptions) ([]linodego.Firewall, error) - DeleteFirewallDevice(ctx context.Context, firewallID, deviceID int) error - ListFirewallDevices(ctx context.Context, firewallID int, opts *linodego.ListOptions) ([]linodego.FirewallDevice, error) ->>>>>>> dadcd59 (fixups + test code from other branch) } // linodego.Client implements Client From aea4bd7fb46e17ee208430749e8e6076b1ec304b Mon Sep 17 00:00:00 2001 From: Tarun Chinmai Sekar Date: Tue, 23 Jan 2024 21:49:42 +0000 Subject: [PATCH 06/19] Comments + lint fix --- cloud/linode/fake_linode_test.go | 11 +++++++---- cloud/linode/loadbalancers.go | 24 +++++++++--------------- cloud/linode/loadbalancers_test.go | 2 +- 3 files changed, 17 insertions(+), 20 deletions(-) diff --git a/cloud/linode/fake_linode_test.go b/cloud/linode/fake_linode_test.go index 43841532..c0483618 100644 --- a/cloud/linode/fake_linode_test.go +++ b/cloud/linode/fake_linode_test.go @@ -24,8 +24,8 @@ type fakeAPI struct { nb map[string]*linodego.NodeBalancer nbc map[string]*linodego.NodeBalancerConfig nbn map[string]*linodego.NodeBalancerNode - fw map[int]*linodego.Firewall - fwd map[int]map[int]*linodego.FirewallDevice + fw map[int]*linodego.Firewall // map of firewallID -> firewall + fwd map[int]map[int]*linodego.FirewallDevice // map of firewallID -> firewallDeviceID:FirewallDevice requests map[fakeRequest]struct{} } @@ -186,12 +186,15 @@ func (f *fakeAPI) ServeHTTP(w http.ResponseWriter, r *http.Request) { }, Data: data, } - rr, _ := json.Marshal(resp) + rr, err := json.Marshal(resp) + if err != nil { + f.t.Fatal(err) + } _, _ = w.Write(rr) return } - rx, _ = regexp.Compile("/nodebalancers/[0-9]+/firewalls") + rx = regexp.MustCompile("/nodebalancers/[0-9]+/firewalls") if rx.MatchString(urlPath) { id := strings.Split(urlPath, "/")[2] devID, err := strconv.Atoi(id) diff --git a/cloud/linode/loadbalancers.go b/cloud/linode/loadbalancers.go index 6359cdb5..2348e9c1 100644 --- a/cloud/linode/loadbalancers.go +++ b/cloud/linode/loadbalancers.go @@ -30,11 +30,6 @@ var ( errTooManyFirewalls = errors.New("Too many firewalls attached to a nodebalancer") ) -const ( - allowListKey = "allowList" - denyListKey = "denyList" -) - type lbNotFoundError struct { serviceNn string nodeBalancerID int @@ -235,8 +230,8 @@ func (l *loadbalancers) EnsureLoadBalancer(ctx context.Context, clusterName stri } // getNodeBalancerDeviceId gets the deviceID of the nodeBalancer that is attached to the firewall. -func (l *loadbalancers) getNodeBalancerDeviceId(ctx context.Context, firewallID, nbID, page int) (int, bool, error) { - devices, err := l.client.ListFirewallDevices(ctx, firewallID, &linodego.ListOptions{PageSize: 500, PageOptions: &linodego.PageOptions{Page: page}}) +func (l *loadbalancers) getNodeBalancerDeviceId(ctx context.Context, firewallID, nbID int) (int, bool, error) { + devices, err := l.client.ListFirewallDevices(ctx, firewallID, &linodego.ListOptions{}) if err != nil { return 0, false, err } @@ -277,10 +272,10 @@ func (l *loadbalancers) updateNodeBalancerFirewall(ctx context.Context, service firewalls, err := l.client.ListNodeBalancerFirewalls(ctx, nb.ID, &linodego.ListOptions{}) if err != nil { if !ok { - if err.Error() != "[404] Not Found" { - return err - } else { + if apiErr, ok := err.(*linodego.Error); ok && apiErr.Code == http.StatusNotFound { return nil + } else { + return err } } } @@ -302,7 +297,7 @@ func (l *loadbalancers) updateNodeBalancerFirewall(ctx context.Context, service // remove the existing firewall if it exists if existingFirewallID != 0 { - deviceID, deviceExists, err := l.getNodeBalancerDeviceId(ctx, existingFirewallID, nb.ID, 1) + deviceID, deviceExists, err := l.getNodeBalancerDeviceId(ctx, existingFirewallID, nb.ID) if err != nil { return err } @@ -377,7 +372,7 @@ func (l *loadbalancers) reconcileFirewall(ctx context.Context, nb *linodego.Node } // We do not want to get into the complexity of reconciling differences, might as well just pull what's in the configMap now and update the fw. - fwCreateOpts, err := l.createFirewallOptsForSvc(ctx, service.Name, []string{""}, service) + fwCreateOpts, err := l.createFirewallOptsForSvc(service.Name, []string{""}, service) if err != nil { return err } @@ -417,7 +412,6 @@ func (l *loadbalancers) updateNodeBalancer(ctx context.Context, clusterName stri } } - // update node-balancer firewall err = l.updateNodeBalancerFirewall(ctx, service, nb) if err != nil { return err @@ -679,7 +673,7 @@ type aclConfig struct { DenyList *linodego.NetworkAddresses `json:"denyList"` } -func (l *loadbalancers) createFirewallOptsForSvc(ctx context.Context, label string, tags []string, svc *v1.Service) (*linodego.FirewallCreateOptions, error) { +func (l *loadbalancers) createFirewallOptsForSvc(label string, tags []string, svc *v1.Service) (*linodego.FirewallCreateOptions, error) { // Fetch acl from annotation aclString := svc.GetAnnotations()[annLinodeCloudFirewallACL] fwcreateOpts := linodego.FirewallCreateOptions{ @@ -741,7 +735,7 @@ func (l *loadbalancers) createNodeBalancer(ctx context.Context, clusterName stri // There's no firewallID already set, see if we need to create a new fw, look for the acl annotation. _, ok := service.GetAnnotations()[annLinodeCloudFirewallACL] if ok { - fwcreateOpts, err := l.createFirewallOptsForSvc(ctx, label, tags, service) + fwcreateOpts, err := l.createFirewallOptsForSvc(label, tags, service) if err != nil { return nil, err } diff --git a/cloud/linode/loadbalancers_test.go b/cloud/linode/loadbalancers_test.go index 268e393a..261312dc 100644 --- a/cloud/linode/loadbalancers_test.go +++ b/cloud/linode/loadbalancers_test.go @@ -740,7 +740,7 @@ func testUpdateLoadBalancerAddTLSPort(t *testing.T, client *linodego.Client, _ * } if !reflect.DeepEqual(expectedPorts, observedPorts) { - t.Errorf("NodeBalancer ports mismatch: expected %v, got %v", `expectedPorts`, observedPorts) + t.Errorf("NodeBalancer ports mismatch: expected %v, got %v", expectedPorts, observedPorts) } } From b5cc4cb2cde3f031000a8c17a5ad1f3c597b7c91 Mon Sep 17 00:00:00 2001 From: Tarun Chinmai Sekar Date: Tue, 23 Jan 2024 23:00:47 +0000 Subject: [PATCH 07/19] Cleanup --- Dockerfile | 6 -- Tiltfile | 94 ------------------ cloud/linode/loadbalancers.go | 176 +++++++++++++++++++--------------- 3 files changed, 97 insertions(+), 179 deletions(-) delete mode 100644 Tiltfile diff --git a/Dockerfile b/Dockerfile index 29126f68..20942766 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,9 +1,3 @@ -# "docker": build in a Docker build container (default) -# "local": copy from the build context. This is useful for tilt's live_update -# feature, allowing hot reload of the linode-ccm binary for fast -# development iteration. See ./Tiltfile -ARG BUILD_SOURCE="docker" - FROM golang:1.21-alpine as builder RUN mkdir -p /linode WORKDIR /linode diff --git a/Tiltfile b/Tiltfile deleted file mode 100644 index 355f50d8..00000000 --- a/Tiltfile +++ /dev/null @@ -1,94 +0,0 @@ -load("ext://deployment", "deployment_create") -load("ext://k8s_attach", "k8s_attach") -load("ext://restart_process", "docker_build_with_restart") -load("ext://helm_resource", "helm_resource", "helm_repo") - -allow_k8s_contexts(k8s_context()) -# helm_repo("registry-repo", "https://helm.twun.io", labels=["helm-repos"]) - -# # in order to use this registry you must add `127.0.0.1 registry-proxy` in your /etc/hosts -# # and add http://registry-proxy:5000 to the list of insecure registries in docker daemon. -load("ext://k8s_attach", "k8s_attach") - -# # Deploy a docker egistry to the cluster, this registry will be used for hosting images built by tilt. -# helm_resource( -# "docker-registry", -# "registry-repo/docker-registry", -# namespace="registry", -# flags=[ -# "--create-namespace", -# "--version=2.2.2", -# "--set=metrics.enabled=true", -# "--set=storage=filesystem", -# #This [documentation](https://docs.docker.com/registry/configuration/#debug) gives more detail about the debug endpoint and how it can be used in production -# #Disabling this will hide the registry metrics on the 5001 port -# ], -# labels=["registry"], -# resource_deps=["registry-repo"], -# ) - -local_resource( - "registry-probe", - serve_cmd="sleep infinity", - readiness_probe=probe( - period_secs=15, - http_get=http_get_action(host="registry-proxy", port=5000, path="/"), - ), - labels=["registry"], - #resource_deps=["docker-registry"], -) - -k8s_attach( - "registry-port-forward", - "deployment/docker-registry", - namespace="registry", - port_forwards=[5000], - labels=["registry"], -) -default_registry("registry-proxy:5000", host_from_cluster="docker.local") - -labels = ["ccm-linode"] - -local_resource( - "linode-cloud-controller-manager-linux-amd64", - "make build-linux", - # No glob support :( - deps=[ - "cloud/", - "go.mod", - "go.sum", - "main.go", - "Makefile", - "sentry/", - ], - ignore=[ - "**/*.bin", - "**/gomock*", - "cloud/linode/mock_client_test.go", - ], - labels=labels, -) - -entrypoint = "/linode-cloud-controller-manager-linux" -docker_build_with_restart( - "linode/linode-cloud-controller-manager", - ".", - entrypoint=[entrypoint], - ignore=["**/*.go", "go.*", "Makefile"], - live_update=[ - sync( - "dist/linode-cloud-controller-manager-linux-amd64", - entrypoint, - ), - ], - build_args={"BUILD_SOURCE": "local"}, - platform="linux/amd64", -) - -chart_yaml = helm( - "deploy/chart", - name="ccm-linode", - namespace="kube-system", - values="./tilt.values.yaml", -) -k8s_yaml(chart_yaml) diff --git a/cloud/linode/loadbalancers.go b/cloud/linode/loadbalancers.go index 2348e9c1..7a05a108 100644 --- a/cloud/linode/loadbalancers.go +++ b/cloud/linode/loadbalancers.go @@ -249,54 +249,35 @@ func (l *loadbalancers) getNodeBalancerDeviceId(ctx context.Context, firewallID, return 0, false, nil } -// updateNodeBalancerFirewall updates the firewall attached to the nodebalancer -// -// Firewall is updated in 3 scenario's: -// - Add new firewall to the nodeBalancer -// - Update firewall attached to the nodebalancer -// - Remove firewall attached -func (l *loadbalancers) updateNodeBalancerFirewall(ctx context.Context, service *v1.Service, nb *linodego.NodeBalancer) error { +func (l *loadbalancers) updateFWwithID(ctx context.Context, service *v1.Service, nb *linodego.NodeBalancer) error { var newFirewallID int var err error - // get the new firewall id form the annotaiton (if any). - fwid, ok := service.GetAnnotations()[annLinodeCloudFirewallID] - if ok { - newFirewallID, err = strconv.Atoi(fwid) - if err != nil { - return err - } + fwID := service.GetAnnotations()[annLinodeCloudFirewallID] + newFirewallID, err = strconv.Atoi(fwID) + if err != nil { + return err } - // get the list of attached firewalls to the node-balancer. + // See if a firewall is attached to the nodebalancer first. firewalls, err := l.client.ListNodeBalancerFirewalls(ctx, nb.ID, &linodego.ListOptions{}) if err != nil { - if !ok { - if apiErr, ok := err.(*linodego.Error); ok && apiErr.Code == http.StatusNotFound { - return nil - } else { - return err - } - } + return err } - - // if there are no attached firewalls and no annotation added; return. - if !ok && len(firewalls) == 0 { - return nil + if len(firewalls) > 1 { + return errTooManyFirewalls } - // get the ID of the firewall that is already attached to the nodeBalancer. + // get the ID of the firewall that is already attached to the nodeBalancer, if we have one. var existingFirewallID int - if len(firewalls) != 0 { + if len(firewalls) == 1 { existingFirewallID = firewalls[0].ID } - // if existing firewall and new firewall differs, update accordingly. + // if existing firewall and new firewall differs, if existingFirewallID != newFirewallID { - // remove the existing firewall if it exists if existingFirewallID != 0 { - deviceID, deviceExists, err := l.getNodeBalancerDeviceId(ctx, existingFirewallID, nb.ID) if err != nil { return err @@ -312,9 +293,40 @@ func (l *loadbalancers) updateNodeBalancerFirewall(ctx context.Context, service } } - // attach new firewall if an ID is provided in the annotation. - if newFirewallID != 0 { - _, err = l.client.CreateFirewallDevice(ctx, newFirewallID, linodego.FirewallDeviceCreateOptions{ + // attach new firewall. + _, err = l.client.CreateFirewallDevice(ctx, newFirewallID, linodego.FirewallDeviceCreateOptions{ + ID: nb.ID, + Type: "nodebalancer", + }) + if err != nil { + return err + } + } + return nil +} + +func (l *loadbalancers) updateFWwithACL(ctx context.Context, service *v1.Service, nb *linodego.NodeBalancer) error { + // See if a firewall is attached to the nodebalancer first. + firewalls, err := l.client.ListNodeBalancerFirewalls(ctx, nb.ID, &linodego.ListOptions{}) + if err != nil { + return err + } + + switch len(firewalls) { + case 0: + { + // need to create a fw and attach it to our nb + fwcreateOpts, err := l.createFirewallOptsForSvc(service.Name, l.getLoadBalancerTags(ctx, "", service), service) + if err != nil { + return err + } + + fw, err := l.client.CreateFirewall(ctx, *fwcreateOpts) + if err != nil { + return err + } + // attach new firewall. + _, err = l.client.CreateFirewallDevice(ctx, fw.ID, linodego.FirewallDeviceCreateOptions{ ID: nb.ID, Type: "nodebalancer", }) @@ -322,39 +334,61 @@ func (l *loadbalancers) updateNodeBalancerFirewall(ctx context.Context, service return err } } + case 1: + { + // We do not want to get into the complexity of reconciling differences, might as well just pull what's in the svc annotation now and update the fw. + fwCreateOpts, err := l.createFirewallOptsForSvc(service.Name, []string{""}, service) + if err != nil { + return err + } + _, err = l.client.UpdateFirewallRules(ctx, firewalls[0].ID, fwCreateOpts.Rules) + if err != nil { + return err + } + } + default: + return errTooManyFirewalls } return nil } -func (l *loadbalancers) reconcileFirewall(ctx context.Context, nb *linodego.NodeBalancer, service *v1.Service) error { - // First things first, see if we need to do anything - // We do that by looking at the fw id annotation, and if it doesn't exist, look at the configMap annotation. - fwid, ok := service.GetAnnotations()[annLinodeCloudFirewallID] - if ok { - _, err := strconv.Atoi(fwid) - if err != nil { - return err - } - // this is a TODO - return nil - } else { - // There's no firewallID already set, see if a acl exists - _, fwACLExists := service.GetAnnotations()[annLinodeCloudFirewallACL] - // Lots to do here. the user may have changed the acl, we should look at what rules exist on the fw first - // and then reconcile it. - firewalls, err := l.client.ListNodeBalancerFirewalls(ctx, nb.ID, &linodego.ListOptions{}) - if err != nil { - return err - } +// updateNodeBalancerFirewall reconciles the firewall attached to the nodebalancer +// +// This function does the follwing +// 1. If a firewallID annotation is present, it checks if the nodebalancer has a firewall attached, and if it matches the annotationID +// a. If the IDs match, nothing to do here. +// b. If they don't match, then the nb is removed from the old FW, and its attached to the new firewall +// 2. If a firewallACL annotation is present, +// a. it checks if the nodebalancer has a firewall attached, if a fw exists, it updates rules +// b. if a fw does not exist, it creates one +// 3. If neither of these annotations are present, +// a. AND if no firewalls are attached to the nodebalancer, nothing to do. +// b. if the NB has ONE firewall attached, remove it from nb, and clean up if nothing else is attached to it +// c. If there are more than one fw attached to it, then its a problem, return an err - if len(firewalls) > 1 { - // we do not support this case. This means that the customer likely attached a different firewall to this nb, - // don't want to risk changing stuff here. - return errTooManyFirewalls - } +func (l *loadbalancers) updateNodeBalancerFirewall(ctx context.Context, service *v1.Service, nb *linodego.NodeBalancer) error { + // get the new firewall id from the annotation (if any). + _, fwIDExists := service.GetAnnotations()[annLinodeCloudFirewallID] + if fwIDExists { // If an ID exists, we ignore everything else and handle just that + return l.updateFWwithID(ctx, service, nb) + } - if !fwACLExists && len(firewalls) == 1 { - // No firewall annotation, but there is a firewall attached our node-balancer. we should remove it. + // See if a acl exists + _, fwACLExists := service.GetAnnotations()[annLinodeCloudFirewallACL] + if fwACLExists { // if an ACL exists, but no ID, just update the ACL on the fw. + return l.updateFWwithACL(ctx, service, nb) + } + + // No firewall ID or ACL annotation, see if there are firewalls attached to our nb + firewalls, err := l.client.ListNodeBalancerFirewalls(ctx, nb.ID, &linodego.ListOptions{}) + if err != nil { + return err + } + switch len(firewalls) { + case 0: + return nil + case 1: + { err := l.client.DeleteFirewallDevice(ctx, firewalls[0].ID, nb.ID) if err != nil { return err @@ -370,16 +404,8 @@ func (l *loadbalancers) reconcileFirewall(ctx context.Context, nb *linodego.Node } // else let that firewall linger, don't mess with it. } - - // We do not want to get into the complexity of reconciling differences, might as well just pull what's in the configMap now and update the fw. - fwCreateOpts, err := l.createFirewallOptsForSvc(service.Name, []string{""}, service) - if err != nil { - return err - } - _, err = l.client.UpdateFirewallRules(ctx, firewalls[0].ID, fwCreateOpts.Rules) - if err != nil { - return err - } + default: + return errTooManyFirewalls } return nil } @@ -489,13 +515,6 @@ func (l *loadbalancers) updateNodeBalancer(ctx context.Context, clusterName stri } } - // Firewall configs - // Delete any configs for ports that have been removed from the Service - if err = l.reconcileFirewall(ctx, nb, service); err != nil { - sentry.CaptureError(ctx, err) - return err - } - return nil } @@ -742,7 +761,6 @@ func (l *loadbalancers) createNodeBalancer(ctx context.Context, clusterName stri firewall, err := l.client.CreateFirewall(ctx, *fwcreateOpts) if err != nil { - fmt.Printf("TARUN here %v", err) return nil, err } createOpts.FirewallID = firewall.ID From 9f4ff579fe0586834151d66b9a713d00aefb4767 Mon Sep 17 00:00:00 2001 From: Tarun Chinmai Sekar Date: Wed, 24 Jan 2024 02:51:23 +0000 Subject: [PATCH 08/19] fixes to test --- cloud/linode/fake_linode_test.go | 2 +- cloud/linode/loadbalancers_test.go | 19 +++++++++++++------ 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/cloud/linode/fake_linode_test.go b/cloud/linode/fake_linode_test.go index c0483618..4e25a9ff 100644 --- a/cloud/linode/fake_linode_test.go +++ b/cloud/linode/fake_linode_test.go @@ -735,7 +735,7 @@ func (f *fakeAPI) ServeHTTP(w http.ResponseWriter, r *http.Request) { func createFirewallDevice(fwId int, f *fakeAPI, fdco linodego.FirewallDeviceCreateOptions) linodego.FirewallDevice { fwd := linodego.FirewallDevice{ - ID: rand.Intn(9999), + ID: fdco.ID, Entity: linodego.FirewallDeviceEntity{ ID: fdco.ID, Type: fdco.Type, diff --git a/cloud/linode/loadbalancers_test.go b/cloud/linode/loadbalancers_test.go index 261312dc..8d6e68dc 100644 --- a/cloud/linode/loadbalancers_test.go +++ b/cloud/linode/loadbalancers_test.go @@ -165,16 +165,26 @@ func TestCCMLoadBalancers(t *testing.T) { f: testUpdateLoadBalancerAddProxyProtocol, }, { - name: "Update Load Balancer - Add new Firewall", + name: "Update Load Balancer - Add new Firewall ID", f: testUpdateLoadBalancerAddNewFirewall, }, { - name: "Update Load Balancer - Update Firewall", + name: "Update Load Balancer - Update Firewall ID", f: testUpdateLoadBalancerUpdateFirewall, }, { - name: "Update Load Balancer - Delete Firewall", + name: "Update Load Balancer - Delete Firewall ID", f: testUpdateLoadBalancerDeleteFirewall, + }, { + name: "Update Load Balancer - Remove Firewall ID & Add ACL", + f: testUpdateLoadBalancerAddNewFirewall, + }, { + name: "Update Load Balancer - Remove both Firewall ID & ACL", + f: testUpdateLoadBalancerAddNewFirewall, + }, + { + name: "Update Load Balancer - Remove both Firewall ACL & Add ID", + f: testUpdateLoadBalancerAddNewFirewall, }, { name: "Build Load Balancer Request", @@ -1091,9 +1101,6 @@ func testUpdateLoadBalancerDeleteFirewall(t *testing.T, client *linodego.Client, ObjectMeta: metav1.ObjectMeta{ Name: randString(), UID: "foobar123", - Annotations: map[string]string{ - annLinodeThrottle: "15", - }, }, Spec: v1.ServiceSpec{ Ports: []v1.ServicePort{ From 862a734686f1a5545e74bafc6d73a5a7674c69d6 Mon Sep 17 00:00:00 2001 From: Tarun Chinmai Sekar Date: Wed, 24 Jan 2024 15:37:55 +0000 Subject: [PATCH 09/19] Lint + tests --- README.md | 53 ++- cloud/linode/fake_linode_test.go | 34 ++ cloud/linode/loadbalancers.go | 11 +- cloud/linode/loadbalancers_test.go | 503 ++++++++++++++++++++++++++++- 4 files changed, 583 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 18204c65..48d65d98 100644 --- a/README.md +++ b/README.md @@ -56,7 +56,8 @@ Annotation (Suffix) | Values | Default | Description `nodebalancer-id` | string | | The ID of the NodeBalancer to front the service. When not specified, a new NodeBalancer will be created. This can be configured on service creation or patching `hostname-only-ingress` | [bool](#annotation-bool-values) | `false` | When `true`, the LoadBalancerStatus for the service will only contain the Hostname. This is useful for bypassing kube-proxy's rerouting of in-cluster requests originally intended for the external LoadBalancer to the service's constituent pod IPs. `tags` | string | | A comma seperated list of tags to be applied to the createad NodeBalancer instance -`firewall-id` | string | | The Firewall ID that's applied to the NodeBalancer instance. +`firewall-id` | string | | An existing Cloud Firewall ID to be attached to the NodeBalancer instance. See [Firewalls](#firewalls). +`firewall-acl` | string | | The Firewall rules to be applied to the NodeBalancer. Adding this annotation creates a new CCM managed Linode CloudFirewall instance. See [Firewalls](#firewalls). #### Deprecated Annotations These annotations are deprecated, and will be removed in a future release. @@ -77,6 +78,56 @@ Key | Values | Default | Description `proxy-protocol` | `none`, `v1`, `v2` | `none` | Specifies whether to use a version of Proxy Protocol on the underlying NodeBalancer. Overwrites `default-proxy-protocol`. `tls-secret-name` | string | | Specifies a secret to use for TLS. The secret type should be `kubernetes.io/tls`. +#### Firewalls +Firewall rules can be applied to the CCM Managed NodeBalancers in two distinct ways. + +##### CCM Managed Firewall +To use this feature, ensure that the linode api token used with the ccm has the `add_firewalls` grant. + +The CCM accepts firewall ACLs in json form. The ACL can either be an `allowList` or a `denyList`. Supplying both is not supported. Supplying neither is not supported. The `allowList` sets up a CloudFirewall that `ACCEPT`s traffic only from the specified IPs/CIDRs and `DROP`s everything else. The `denyList` sets up a CloudFirewall that `DROP`s traffic only from the specified IPs/CIDRs and `ACCEPT`s everything else. Ports are automatically inferred from the service configuration. + +See [Firewall rules](https://www.linode.com/docs/api/networking/#firewall-create__request-body-schema) for more details on how to specify the IPs/CIDRs + +Example usage of an ACL to allow traffic from a specific set of addresses + +```yaml +kind: Service +apiVersion: v1 +metadata: + name: https-lb + annotations: + service.beta.kubernetes.io/linode-loadbalancer-firewall-acl: | + { + "allowList": { + "ipv4": ["192.166.0.0/16", "172.23.41.0/24"], + "ipv6": ["2001:DB8::/128"] + }, + } +spec: + type: LoadBalancer + selector: + app: nginx-https-example + ports: + - name: http + protocol: TCP + port: 80 + targetPort: http + - name: https + protocol: TCP + port: 443 + targetPort: https +``` + + +##### User Managed Firewall +Users can create CloudFirewall instances, supply their own rules and attach them to the NodeBalancer. To do so, set the +`service.beta.kubernetes.io/linode-loadbalancer-firewall-id` annotation to the ID of the cloud firewall. The CCM does not manage the lifecycle of the CloudFirewall Instance in this case. Users are responsible for ensuring the policies are correct. + +**Note**
+If the user supplies a firewall-id, and later switches to using an ACL, the CCM will take over the CloudFirewall Instance. To avoid this, delete the service, and re-create it so the original CloudFirewall is left undisturbed. + + + ### Nodes Kubernetes Nodes can be configured with the following annotations. diff --git a/cloud/linode/fake_linode_test.go b/cloud/linode/fake_linode_test.go index 4e25a9ff..d3a37e61 100644 --- a/cloud/linode/fake_linode_test.go +++ b/cloud/linode/fake_linode_test.go @@ -729,6 +729,40 @@ func (f *fakeAPI) ServeHTTP(w http.ResponseWriter, r *http.Request) { rr, _ := json.Marshal(resp) _, _ = w.Write(rr) + } else if strings.Contains(urlPath, "firewalls") { + // path is networking/firewalls/%d/rules + parts := strings.Split(urlPath[1:], "/") + fwrs := new(linodego.FirewallRuleSet) + if err := json.NewDecoder(r.Body).Decode(fwrs); err != nil { + f.t.Fatal(err) + } + + fwID, err := strconv.Atoi(parts[2]) + if err != nil { + f.t.Fatal(err) + } + + if firewall, found := f.fw[fwID]; found { + firewall.Rules.Inbound = fwrs.Inbound + firewall.Rules.InboundPolicy = fwrs.InboundPolicy + // outbound rules do not apply, ignoring. + f.fw[fwID] = firewall + resp, err := json.Marshal(firewall) + if err != nil { + f.t.Fatal(err) + } + _, _ = w.Write(resp) + return + } + + w.WriteHeader(404) + resp := linodego.APIError{ + Errors: []linodego.APIErrorReason{ + {Reason: "Not Found"}, + }, + } + rr, _ := json.Marshal(resp) + _, _ = w.Write(rr) } } } diff --git a/cloud/linode/loadbalancers.go b/cloud/linode/loadbalancers.go index 7a05a108..778ca020 100644 --- a/cloud/linode/loadbalancers.go +++ b/cloud/linode/loadbalancers.go @@ -365,6 +365,7 @@ func (l *loadbalancers) updateFWwithACL(ctx context.Context, service *v1.Service // a. AND if no firewalls are attached to the nodebalancer, nothing to do. // b. if the NB has ONE firewall attached, remove it from nb, and clean up if nothing else is attached to it // c. If there are more than one fw attached to it, then its a problem, return an err +// IF a user creates a fw ID externally, and then switches to using a ACL, the CCM will take over the fw that's attached to the nodebalancer. func (l *loadbalancers) updateNodeBalancerFirewall(ctx context.Context, service *v1.Service, nb *linodego.NodeBalancer) error { // get the new firewall id from the annotation (if any). @@ -667,7 +668,7 @@ func (l *loadbalancers) getLoadBalancerTags(_ context.Context, clusterName strin return tags } -func processACL(fwcreateOpts *linodego.FirewallCreateOptions, aclType, label, svcName, ports string, ips linodego.NetworkAddresses) error { +func processACL(fwcreateOpts *linodego.FirewallCreateOptions, aclType, label, svcName, ports string, ips linodego.NetworkAddresses) { fwcreateOpts.Rules.Inbound = append(fwcreateOpts.Rules.Inbound, linodego.FirewallRule{ Action: aclType, Label: fmt.Sprintf("%s-%s", aclType, svcName), @@ -684,7 +685,6 @@ func processACL(fwcreateOpts *linodego.FirewallCreateOptions, aclType, label, sv // if a denylist is present, we accept everything else. fwcreateOpts.Rules.InboundPolicy = "ACCEPT" } - return nil } type aclConfig struct { @@ -723,11 +723,8 @@ func (l *loadbalancers) createFirewallOptsForSvc(label string, tags []string, sv allowedIPs = acl.DenyList } - err = processACL(&fwcreateOpts, aclType, label, svc.Name, portsString, *allowedIPs) - if err != nil { - return nil, err - } - return &fwcreateOpts, err + processACL(&fwcreateOpts, aclType, label, svc.Name, portsString, *allowedIPs) + return &fwcreateOpts, nil } func (l *loadbalancers) createNodeBalancer(ctx context.Context, clusterName string, service *v1.Service, configs []*linodego.NodeBalancerConfigCreateOptions) (lb *linodego.NodeBalancer, err error) { diff --git a/cloud/linode/loadbalancers_test.go b/cloud/linode/loadbalancers_test.go index 8d6e68dc..858f3171 100644 --- a/cloud/linode/loadbalancers_test.go +++ b/cloud/linode/loadbalancers_test.go @@ -133,10 +133,12 @@ func TestCCMLoadBalancers(t *testing.T) { { name: "Create Load Balancer With Valid Firewall ACL - DenyList", f: testCreateNodeBalancerWithDenyList, - }, { + }, + { name: "Create Load Balancer With Invalid Firewall ACL - Both Allow and Deny", f: testCreateNodeBalanceWithBothAllowOrDenyList, - }, { + }, + { name: "Create Load Balancer With Invalid Firewall ACL - NO Allow Or Deny", f: testCreateNodeBalanceWithNoAllowOrDenyList, }, @@ -175,16 +177,22 @@ func TestCCMLoadBalancers(t *testing.T) { { name: "Update Load Balancer - Delete Firewall ID", f: testUpdateLoadBalancerDeleteFirewall, - }, { + }, + { + name: "Update Load Balancer - Update Firewall ACL", + f: testUpdateLoadBalancerUpdateFirewallACL, + }, + { name: "Update Load Balancer - Remove Firewall ID & Add ACL", - f: testUpdateLoadBalancerAddNewFirewall, - }, { - name: "Update Load Balancer - Remove both Firewall ID & ACL", - f: testUpdateLoadBalancerAddNewFirewall, + f: testUpdateLoadBalancerUpdateFirewallRemoveIDaddACL, }, { - name: "Update Load Balancer - Remove both Firewall ACL & Add ID", - f: testUpdateLoadBalancerAddNewFirewall, + name: "Update Load Balancer - Remove Firewall ACL & Add ID", + f: testUpdateLoadBalancerUpdateFirewallRemoveACLaddID, + }, + { + name: "Update Load Balancer - Add a new Firewall ACL", + f: testUpdateLoadBalancerAddNewFirewallACL, }, { name: "Build Load Balancer Request", @@ -390,7 +398,6 @@ func testCreateNodeBalancerWithAllowList(t *testing.T, client *linodego.Client, if err != nil { t.Fatalf("expected a non-nil error, got %v", err) } - } func testCreateNodeBalancerWithDenyList(t *testing.T, client *linodego.Client, f *fakeAPI) { @@ -953,6 +960,482 @@ func testUpdateLoadBalancerAddNewFirewall(t *testing.T, client *linodego.Client, } } +func testUpdateLoadBalancerAddNewFirewallACL(t *testing.T, client *linodego.Client, fakeAPI *fakeAPI) { + svc := &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: randString(), + UID: "foobar123", + }, + Spec: v1.ServiceSpec{ + Ports: []v1.ServicePort{ + { + Name: randString(), + Protocol: "TCP", + Port: int32(80), + NodePort: int32(30000), + }, + }, + }, + } + + nodes := []*v1.Node{ + { + Status: v1.NodeStatus{ + Addresses: []v1.NodeAddress{ + { + Type: v1.NodeInternalIP, + Address: "127.0.0.1", + }, + }, + }, + }, + } + + lb := &loadbalancers{client, "us-west", nil} + fakeClientset := fake.NewSimpleClientset() + lb.kubeClient = fakeClientset + + defer func() { + _ = lb.EnsureLoadBalancerDeleted(context.TODO(), "linodelb", svc) + }() + lbStatus, err := lb.EnsureLoadBalancer(context.TODO(), "linodelb", svc, nodes) + if err != nil { + t.Errorf("EnsureLoadBalancer returned an error: %s", err) + } + svc.Status.LoadBalancer = *lbStatus + stubService(fakeClientset, svc) + + nb, err := lb.getNodeBalancerByStatus(context.TODO(), svc) + if err != nil { + t.Fatalf("failed to get NodeBalancer via status: %s", err) + } + + firewalls, err := lb.client.ListNodeBalancerFirewalls(context.TODO(), nb.ID, &linodego.ListOptions{}) + if err != nil { + t.Fatalf("Failed to list nodeBalancer firewalls %s", err) + } + + if len(firewalls) != 0 { + t.Fatalf("Firewalls attached when none specified") + } + + svc.ObjectMeta.SetAnnotations(map[string]string{ + annLinodeCloudFirewallACL: `{ + "allowList": { + "ipv4": ["2.2.2.2"] + } + }`, + }) + + err = lb.UpdateLoadBalancer(context.TODO(), "linodelb", svc, nodes) + if err != nil { + t.Errorf("UpdateLoadBalancer returned an error: %s", err) + } + + nbUpdated, err := lb.getNodeBalancerByStatus(context.TODO(), svc) + if err != nil { + t.Fatalf("failed to get NodeBalancer via status: %s", err) + } + + firewallsNew, err := lb.client.ListNodeBalancerFirewalls(context.TODO(), nbUpdated.ID, &linodego.ListOptions{}) + if err != nil { + t.Fatalf("failed to List Firewalls %s", err) + } + + if len(firewallsNew) == 0 { + t.Fatalf("No firewalls found") + } + + if firewallsNew[0].Rules.InboundPolicy != "DROP" { + t.Errorf("expected DROP inbound policy, got %s", firewallsNew[0].Rules.InboundPolicy) + } + + fwIPs := firewallsNew[0].Rules.Inbound[0].Addresses.IPv4 + if fwIPs == nil { + t.Errorf("expected 2.2.2.2, got %v", fwIPs) + } +} + +func testUpdateLoadBalancerUpdateFirewallRemoveACLaddID(t *testing.T, client *linodego.Client, fakeAPI *fakeAPI) { + svc := &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: randString(), + UID: "foobar123", + }, + Spec: v1.ServiceSpec{ + Ports: []v1.ServicePort{ + { + Name: randString(), + Protocol: "TCP", + Port: int32(80), + NodePort: int32(30000), + }, + }, + }, + } + + nodes := []*v1.Node{ + { + Status: v1.NodeStatus{ + Addresses: []v1.NodeAddress{ + { + Type: v1.NodeInternalIP, + Address: "127.0.0.1", + }, + }, + }, + }, + } + + lb := &loadbalancers{client, "us-west", nil} + fakeClientset := fake.NewSimpleClientset() + lb.kubeClient = fakeClientset + + svc.ObjectMeta.SetAnnotations(map[string]string{ + annLinodeCloudFirewallACL: `{ + "allowList": { + "ipv4": ["2.2.2.2"] + } + }`, + }) + + defer func() { + _ = lb.EnsureLoadBalancerDeleted(context.TODO(), "linodelb", svc) + }() + lbStatus, err := lb.EnsureLoadBalancer(context.TODO(), "linodelb", svc, nodes) + if err != nil { + t.Errorf("EnsureLoadBalancer returned an error: %s", err) + } + svc.Status.LoadBalancer = *lbStatus + stubService(fakeClientset, svc) + + nb, err := lb.getNodeBalancerByStatus(context.TODO(), svc) + if err != nil { + t.Fatalf("failed to get NodeBalancer via status: %s", err) + } + + firewalls, err := lb.client.ListNodeBalancerFirewalls(context.TODO(), nb.ID, &linodego.ListOptions{}) + if err != nil { + t.Fatalf("Failed to list nodeBalancer firewalls %s", err) + } + + if len(firewalls) == 0 { + t.Fatalf("No firewalls attached") + } + + if firewalls[0].Rules.InboundPolicy != "DROP" { + t.Errorf("expected DROP inbound policy, got %s", firewalls[0].Rules.InboundPolicy) + } + + fwIPs := firewalls[0].Rules.Inbound[0].Addresses.IPv4 + if fwIPs == nil { + t.Errorf("expected IP, got %v", fwIPs) + } + + firewall, err := lb.createFirewall(context.TODO(), linodego.FirewallCreateOptions{ + Label: "test", + Rules: linodego.FirewallRuleSet{Inbound: []linodego.FirewallRule{{ + Action: "ACCEPT", + Label: "inbound-rule123", + Description: "inbound rule123", + Ports: "4321", + Protocol: linodego.TCP, + Addresses: linodego.NetworkAddresses{ + IPv4: &[]string{"0.0.0.0/0"}, + }, + }}, Outbound: []linodego.FirewallRule{}, InboundPolicy: "ACCEPT", OutboundPolicy: "ACCEPT"}, + }) + if err != nil { + t.Errorf("Error creating firewall %s", err) + } + defer func() { + _ = lb.deleteFirewall(context.TODO(), firewall) + }() + + svc.ObjectMeta.SetAnnotations(map[string]string{ + annLinodeCloudFirewallID: strconv.Itoa(firewall.ID), + }) + + err = lb.UpdateLoadBalancer(context.TODO(), "linodelb", svc, nodes) + if err != nil { + t.Errorf("UpdateLoadBalancer returned an error: %s", err) + } + + nbUpdated, err := lb.getNodeBalancerByStatus(context.TODO(), svc) + if err != nil { + t.Fatalf("failed to get NodeBalancer via status: %s", err) + } + + firewallsNew, err := lb.client.ListNodeBalancerFirewalls(context.TODO(), nbUpdated.ID, &linodego.ListOptions{}) + if err != nil { + t.Fatalf("failed to List Firewalls %s", err) + } + + if len(firewallsNew) == 0 { + t.Fatalf("No attached firewalls found") + } + + if firewallsNew[0].Rules.InboundPolicy != "ACCEPT" { + t.Errorf("expected ACCEPT inbound policy, got %s", firewallsNew[0].Rules.InboundPolicy) + } + + fwIPs = firewallsNew[0].Rules.Inbound[0].Addresses.IPv4 + if fwIPs == nil { + t.Errorf("expected 2.2.2.2, got %v", fwIPs) + } + + if firewallsNew[0].ID != firewall.ID { + t.Errorf("Firewall ID does not match what we created, something wrong.") + } +} + +func testUpdateLoadBalancerUpdateFirewallRemoveIDaddACL(t *testing.T, client *linodego.Client, fakeAPI *fakeAPI) { + svc := &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: randString(), + UID: "foobar123", + }, + Spec: v1.ServiceSpec{ + Ports: []v1.ServicePort{ + { + Name: randString(), + Protocol: "TCP", + Port: int32(80), + NodePort: int32(30000), + }, + }, + }, + } + + nodes := []*v1.Node{ + { + Status: v1.NodeStatus{ + Addresses: []v1.NodeAddress{ + { + Type: v1.NodeInternalIP, + Address: "127.0.0.1", + }, + }, + }, + }, + } + + lb := &loadbalancers{client, "us-west", nil} + fakeClientset := fake.NewSimpleClientset() + lb.kubeClient = fakeClientset + + firewall, err := lb.createFirewall(context.TODO(), linodego.FirewallCreateOptions{ + Label: "test", + Rules: linodego.FirewallRuleSet{Inbound: []linodego.FirewallRule{{ + Action: "ACCEPT", + Label: "inbound-rule123", + Description: "inbound rule123", + Ports: "4321", + Protocol: linodego.TCP, + Addresses: linodego.NetworkAddresses{ + IPv4: &[]string{"0.0.0.0/0"}, + }, + }}, Outbound: []linodego.FirewallRule{}, InboundPolicy: "ACCEPT", OutboundPolicy: "ACCEPT"}, + }) + if err != nil { + t.Errorf("Error creating firewall %s", err) + } + defer func() { + _ = lb.deleteFirewall(context.TODO(), firewall) + }() + + svc.ObjectMeta.SetAnnotations(map[string]string{ + annLinodeCloudFirewallID: strconv.Itoa(firewall.ID), + }) + + defer func() { + _ = lb.EnsureLoadBalancerDeleted(context.TODO(), "linodelb", svc) + }() + lbStatus, err := lb.EnsureLoadBalancer(context.TODO(), "linodelb", svc, nodes) + if err != nil { + t.Errorf("EnsureLoadBalancer returned an error: %s", err) + } + svc.Status.LoadBalancer = *lbStatus + stubService(fakeClientset, svc) + + nb, err := lb.getNodeBalancerByStatus(context.TODO(), svc) + if err != nil { + t.Fatalf("failed to get NodeBalancer via status: %s", err) + } + + firewalls, err := lb.client.ListNodeBalancerFirewalls(context.TODO(), nb.ID, &linodego.ListOptions{}) + if err != nil { + t.Fatalf("Failed to list nodeBalancer firewalls %s", err) + } + + if len(firewalls) == 0 { + t.Fatalf("No firewalls attached") + } + + if firewalls[0].Rules.InboundPolicy != "ACCEPT" { + t.Errorf("expected ACCEPT inbound policy, got %s", firewalls[0].Rules.InboundPolicy) + } + + fwIPs := firewalls[0].Rules.Inbound[0].Addresses.IPv4 + if fwIPs == nil { + t.Errorf("expected IP, got %v", fwIPs) + } + fmt.Printf("TARUN Old %v\n", firewalls) + svc.ObjectMeta.SetAnnotations(map[string]string{ + annLinodeCloudFirewallACL: `{ + "allowList": { + "ipv4": ["2.2.2.2"] + } + }`, + }) + + err = lb.UpdateLoadBalancer(context.TODO(), "linodelb", svc, nodes) + if err != nil { + t.Errorf("UpdateLoadBalancer returned an error: %s", err) + } + + nbUpdated, err := lb.getNodeBalancerByStatus(context.TODO(), svc) + if err != nil { + t.Fatalf("failed to get NodeBalancer via status: %s", err) + } + + firewallsNew, err := lb.client.ListNodeBalancerFirewalls(context.TODO(), nbUpdated.ID, &linodego.ListOptions{}) + if err != nil { + t.Fatalf("failed to List Firewalls %s", err) + } + + if len(firewallsNew) == 0 { + t.Fatalf("No attached firewalls found") + } + + if firewallsNew[0].Rules.InboundPolicy != "DROP" { + t.Errorf("expected DROP inbound policy, got %s", firewallsNew[0].Rules.InboundPolicy) + } + + fwIPs = firewallsNew[0].Rules.Inbound[0].Addresses.IPv4 + if fwIPs == nil { + t.Errorf("expected 2.2.2.2, got %v", fwIPs) + } + + if firewallsNew[0].ID != firewall.ID { + t.Errorf("Firewall ID does not match, something wrong.") + } +} + +func testUpdateLoadBalancerUpdateFirewallACL(t *testing.T, client *linodego.Client, fakeAPI *fakeAPI) { + svc := &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: randString(), + UID: "foobar123", + Annotations: map[string]string{ + annLinodeCloudFirewallACL: `{ + "allowList": { + "ipv4": ["2.2.2.2"] + } + }`, + }, + }, + Spec: v1.ServiceSpec{ + Ports: []v1.ServicePort{ + { + Name: randString(), + Protocol: "TCP", + Port: int32(80), + NodePort: int32(30000), + }, + }, + }, + } + + nodes := []*v1.Node{ + { + Status: v1.NodeStatus{ + Addresses: []v1.NodeAddress{ + { + Type: v1.NodeInternalIP, + Address: "127.0.0.1", + }, + }, + }, + }, + } + + lb := &loadbalancers{client, "us-west", nil} + fakeClientset := fake.NewSimpleClientset() + lb.kubeClient = fakeClientset + + defer func() { + _ = lb.EnsureLoadBalancerDeleted(context.TODO(), "linodelb", svc) + }() + lbStatus, err := lb.EnsureLoadBalancer(context.TODO(), "linodelb", svc, nodes) + if err != nil { + t.Errorf("EnsureLoadBalancer returned an error: %s", err) + } + svc.Status.LoadBalancer = *lbStatus + stubService(fakeClientset, svc) + + nb, err := lb.getNodeBalancerByStatus(context.TODO(), svc) + if err != nil { + t.Fatalf("failed to get NodeBalancer via status: %s", err) + } + + firewalls, err := lb.client.ListNodeBalancerFirewalls(context.TODO(), nb.ID, &linodego.ListOptions{}) + if err != nil { + t.Fatalf("Failed to list nodeBalancer firewalls %s", err) + } + + if len(firewalls) == 0 { + t.Fatalf("No firewalls attached") + } + + if firewalls[0].Rules.InboundPolicy != "DROP" { + t.Errorf("expected DROP inbound policy, got %s", firewalls[0].Rules.InboundPolicy) + } + + fwIPs := firewalls[0].Rules.Inbound[0].Addresses.IPv4 + if fwIPs == nil { + t.Errorf("expected 2.2.2.2, got %v", fwIPs) + } + + fmt.Printf("got %v", fwIPs) + + svc.ObjectMeta.SetAnnotations(map[string]string{ + annLinodeCloudFirewallACL: `{ + "denyList": { + "ipv4": ["2.2.2.2"] + } + }`, + }) + + err = lb.UpdateLoadBalancer(context.TODO(), "linodelb", svc, nodes) + if err != nil { + t.Errorf("UpdateLoadBalancer returned an error: %s", err) + } + + nbUpdated, err := lb.getNodeBalancerByStatus(context.TODO(), svc) + if err != nil { + t.Fatalf("failed to get NodeBalancer via status: %s", err) + } + + firewallsNew, err := lb.client.ListNodeBalancerFirewalls(context.TODO(), nbUpdated.ID, &linodego.ListOptions{}) + if err != nil { + t.Fatalf("failed to List Firewalls %s", err) + } + + if len(firewallsNew) == 0 { + t.Fatalf("No attached firewalls found") + } + + if firewallsNew[0].Rules.InboundPolicy != "ACCEPT" { + t.Errorf("expected ACCEPT inbound policy, got %s", firewallsNew[0].Rules.InboundPolicy) + } + + fwIPs = firewallsNew[0].Rules.Inbound[0].Addresses.IPv4 + if fwIPs == nil { + t.Errorf("expected 2.2.2.2, got %v", fwIPs) + } +} + func testUpdateLoadBalancerUpdateFirewall(t *testing.T, client *linodego.Client, fakeAPI *fakeAPI) { firewallCreateOpts := linodego.FirewallCreateOptions{ Label: "test", From 67ae0afefe363765a5ecce17ea7f1891b819113e Mon Sep 17 00:00:00 2001 From: Tarun Chinmai Sekar Date: Wed, 24 Jan 2024 15:40:16 +0000 Subject: [PATCH 10/19] Remove unnneccessary changes --- deploy/chart/templates/clusterrole-rbac.yaml | 3 -- deploy/chart/values.yaml | 2 +- examples/http-nginx-firewalled.yaml | 47 ++++++++++++++++++++ examples/http-nginx.yaml | 8 ---- 4 files changed, 48 insertions(+), 12 deletions(-) create mode 100644 examples/http-nginx-firewalled.yaml diff --git a/deploy/chart/templates/clusterrole-rbac.yaml b/deploy/chart/templates/clusterrole-rbac.yaml index 588f51ec..29532135 100644 --- a/deploy/chart/templates/clusterrole-rbac.yaml +++ b/deploy/chart/templates/clusterrole-rbac.yaml @@ -21,9 +21,6 @@ rules: - apiGroups: [""] resources: ["secrets"] verbs: ["get"] - - apiGroups: [""] - resources: ["configmaps"] - verbs: ["get"] - apiGroups: [""] resources: ["services"] verbs: ["get", "watch", "list"] diff --git a/deploy/chart/values.yaml b/deploy/chart/values.yaml index 7b2e3804..790458df 100644 --- a/deploy/chart/values.yaml +++ b/deploy/chart/values.yaml @@ -48,4 +48,4 @@ tolerations: # LINODE_HOSTNAME_ONLY_INGRESS type bool is supported # env: # - name: EXAMPLE_ENV_VAR - # value: "true" + # value: "true" \ No newline at end of file diff --git a/examples/http-nginx-firewalled.yaml b/examples/http-nginx-firewalled.yaml new file mode 100644 index 00000000..0bf70efd --- /dev/null +++ b/examples/http-nginx-firewalled.yaml @@ -0,0 +1,47 @@ +--- +kind: Service +apiVersion: v1 +metadata: + name: http-lb + namespace: kube-system + annotations: + service.beta.kubernetes.io/linode-loadbalancer-default-protocol: "http" + service.beta.kubernetes.io/linode-loadbalancer-firewall-acl: | + { + "allowList": { + "ipv4": ["8.8.8.8/32"], + "ipv6": ["dead:beef::/64"] + } + } +spec: + type: LoadBalancer + selector: + app: nginx-http-example + ports: + - name: http + protocol: TCP + port: 80 + targetPort: 80 + +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-http-deployment + namespace: kube-system +spec: + replicas: 2 + selector: + matchLabels: + app: nginx-http-example + template: + metadata: + labels: + app: nginx-http-example + spec: + containers: + - name: nginx + image: nginx + ports: + - containerPort: 80 + protocol: TCP diff --git a/examples/http-nginx.yaml b/examples/http-nginx.yaml index 0bf70efd..b9d351e9 100644 --- a/examples/http-nginx.yaml +++ b/examples/http-nginx.yaml @@ -3,16 +3,8 @@ kind: Service apiVersion: v1 metadata: name: http-lb - namespace: kube-system annotations: service.beta.kubernetes.io/linode-loadbalancer-default-protocol: "http" - service.beta.kubernetes.io/linode-loadbalancer-firewall-acl: | - { - "allowList": { - "ipv4": ["8.8.8.8/32"], - "ipv6": ["dead:beef::/64"] - } - } spec: type: LoadBalancer selector: From d4dcba7d9417bbe025cf9c63f23ad22e0b6512d5 Mon Sep 17 00:00:00 2001 From: Tarun Chinmai Sekar Date: Wed, 24 Jan 2024 15:40:48 +0000 Subject: [PATCH 11/19] remove change --- examples/http-nginx.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/examples/http-nginx.yaml b/examples/http-nginx.yaml index b9d351e9..43312bb9 100644 --- a/examples/http-nginx.yaml +++ b/examples/http-nginx.yaml @@ -20,7 +20,6 @@ apiVersion: apps/v1 kind: Deployment metadata: name: nginx-http-deployment - namespace: kube-system spec: replicas: 2 selector: From 94e0399fc70d6fd5be6ac00a739545f9a21ea98d Mon Sep 17 00:00:00 2001 From: Tarun Chinmai Sekar Date: Wed, 24 Jan 2024 23:27:41 +0000 Subject: [PATCH 12/19] fixes --- cloud/linode/loadbalancers.go | 6 +++++- cloud/linode/loadbalancers_test.go | 1 - 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/cloud/linode/loadbalancers.go b/cloud/linode/loadbalancers.go index 778ca020..9a0b3066 100644 --- a/cloud/linode/loadbalancers.go +++ b/cloud/linode/loadbalancers.go @@ -660,7 +660,11 @@ func (l *loadbalancers) getNodeBalancerByID(ctx context.Context, service *v1.Ser } func (l *loadbalancers) getLoadBalancerTags(_ context.Context, clusterName string, service *v1.Service) []string { - tags := []string{clusterName} + tags := []string{} + if clusterName != "" { + tags = append(tags, clusterName) + } + tagStr, ok := service.GetAnnotations()[annLinodeLoadBalancerTags] if ok { return append(tags, strings.Split(tagStr, ",")...) diff --git a/cloud/linode/loadbalancers_test.go b/cloud/linode/loadbalancers_test.go index 858f3171..44d0249d 100644 --- a/cloud/linode/loadbalancers_test.go +++ b/cloud/linode/loadbalancers_test.go @@ -1280,7 +1280,6 @@ func testUpdateLoadBalancerUpdateFirewallRemoveIDaddACL(t *testing.T, client *li if fwIPs == nil { t.Errorf("expected IP, got %v", fwIPs) } - fmt.Printf("TARUN Old %v\n", firewalls) svc.ObjectMeta.SetAnnotations(map[string]string{ annLinodeCloudFirewallACL: `{ "allowList": { From 6d44fe427cd6ea913a8d1a4f073b0749e3a1baf0 Mon Sep 17 00:00:00 2001 From: Tarun Chinmai Sekar Date: Thu, 25 Jan 2024 01:52:17 +0000 Subject: [PATCH 13/19] unique fw name --- cloud/linode/loadbalancers.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cloud/linode/loadbalancers.go b/cloud/linode/loadbalancers.go index 9a0b3066..aeb0ebad 100644 --- a/cloud/linode/loadbalancers.go +++ b/cloud/linode/loadbalancers.go @@ -316,7 +316,7 @@ func (l *loadbalancers) updateFWwithACL(ctx context.Context, service *v1.Service case 0: { // need to create a fw and attach it to our nb - fwcreateOpts, err := l.createFirewallOptsForSvc(service.Name, l.getLoadBalancerTags(ctx, "", service), service) + fwcreateOpts, err := l.createFirewallOptsForSvc(l.GetLoadBalancerName(ctx, "", service), l.getLoadBalancerTags(ctx, "", service), service) if err != nil { return err } From 817ade0a13168922de4dd8680e5d7ae18e530ea3 Mon Sep 17 00:00:00 2001 From: Tarun Chinmai Sekar Date: Thu, 25 Jan 2024 17:00:02 +0000 Subject: [PATCH 14/19] Review comments --- cloud/linode/loadbalancers.go | 161 ++++++++++++++++++++++------- cloud/linode/loadbalancers_test.go | 23 +++-- 2 files changed, 141 insertions(+), 43 deletions(-) diff --git a/cloud/linode/loadbalancers.go b/cloud/linode/loadbalancers.go index aeb0ebad..22dc1ac1 100644 --- a/cloud/linode/loadbalancers.go +++ b/cloud/linode/loadbalancers.go @@ -8,6 +8,7 @@ import ( "net/http" "os" "reflect" + "slices" "strconv" "strings" "time" @@ -249,7 +250,11 @@ func (l *loadbalancers) getNodeBalancerDeviceId(ctx context.Context, firewallID, return 0, false, nil } -func (l *loadbalancers) updateFWwithID(ctx context.Context, service *v1.Service, nb *linodego.NodeBalancer) error { +// Updates a service that has a firewallID annotation set. +// If an annotation is set, and the nodebalancer has a firewall that matches the ID, nothing to do +// If there's more than one firewall attached to the node-balancer, an error is returned as its not a supported use case. +// If there's only one firewall attached and it doesn't match what's in the annotation, the new firewall is attached and the old one removed +func (l *loadbalancers) updateFirewallwithID(ctx context.Context, service *v1.Service, nb *linodego.NodeBalancer) error { var newFirewallID int var err error @@ -274,8 +279,16 @@ func (l *loadbalancers) updateFWwithID(ctx context.Context, service *v1.Service, existingFirewallID = firewalls[0].ID } - // if existing firewall and new firewall differs, + // if existing firewall and new firewall differs, attach the new firewall and remove the old. if existingFirewallID != newFirewallID { + // attach new firewall. + _, err = l.client.CreateFirewallDevice(ctx, newFirewallID, linodego.FirewallDeviceCreateOptions{ + ID: nb.ID, + Type: "nodebalancer", + }) + if err != nil { + return err + } // remove the existing firewall if it exists if existingFirewallID != 0 { deviceID, deviceExists, err := l.getNodeBalancerDeviceId(ctx, existingFirewallID, nb.ID) @@ -292,17 +305,79 @@ func (l *loadbalancers) updateFWwithID(ctx context.Context, service *v1.Service, return err } } + } + return nil +} - // attach new firewall. - _, err = l.client.CreateFirewallDevice(ctx, newFirewallID, linodego.FirewallDeviceCreateOptions{ - ID: nb.ID, - Type: "nodebalancer", - }) - if err != nil { - return err +func ipsChanged(ips *linodego.NetworkAddresses, rules []linodego.FirewallRule) bool { + var ruleIPv4s []string + var ruleIPv6s []string + + for _, rule := range rules { + if rule.Addresses.IPv4 != nil { + for _, ip := range *rule.Addresses.IPv4 { + ruleIPv4s = append(ruleIPv4s, ip) + } + } + if rule.Addresses.IPv6 != nil { + for _, ip := range *rule.Addresses.IPv4 { + ruleIPv6s = append(ruleIPv6s, ip) + } } } - return nil + + if len(ruleIPv4s) > 0 && ips.IPv4 == nil { + return true + } + + if len(ruleIPv6s) > 0 && ips.IPv6 == nil { + return true + } + + if ips.IPv4 != nil { + for _, ipv4 := range *ips.IPv4 { + if !slices.Contains(ruleIPv4s, ipv4) { + return true + } + } + } + + if ips.IPv6 != nil { + for _, ipv6 := range *ips.IPv6 { + if !slices.Contains(ruleIPv6s, ipv6) { + return true + } + } + } + + return false +} + +func firewallRuleChanged(old linodego.FirewallRuleSet, newACL aclConfig) bool { + var ips *linodego.NetworkAddresses + if newACL.AllowList != nil { + // this is a allowList, this means that the rules should have `DROP` as inboundpolicy + if old.InboundPolicy != "DROP" { + return true + } + if (newACL.AllowList.IPv4 != nil || newACL.AllowList.IPv6 != nil) && len(old.Inbound) == 0 { + return true + } + ips = newACL.AllowList + } + + if newACL.DenyList != nil { + if old.InboundPolicy != "ACCEPT" { + return true + } + + if (newACL.DenyList.IPv4 != nil || newACL.DenyList.IPv6 != nil) && len(old.Inbound) == 0 { + return true + } + ips = newACL.DenyList + } + + return ipsChanged(ips, old.Inbound) } func (l *loadbalancers) updateFWwithACL(ctx context.Context, service *v1.Service, nb *linodego.NodeBalancer) error { @@ -337,6 +412,17 @@ func (l *loadbalancers) updateFWwithACL(ctx context.Context, service *v1.Service case 1: { // We do not want to get into the complexity of reconciling differences, might as well just pull what's in the svc annotation now and update the fw. + var acl aclConfig + err := json.Unmarshal([]byte(service.GetAnnotations()[annLinodeCloudFirewallACL]), &acl) + if err != nil { + return err + } + + changed := firewallRuleChanged(firewalls[0].Rules, acl) + if !changed { + return nil + } + fwCreateOpts, err := l.createFirewallOptsForSvc(service.Name, []string{""}, service) if err != nil { return err @@ -354,10 +440,10 @@ func (l *loadbalancers) updateFWwithACL(ctx context.Context, service *v1.Service // updateNodeBalancerFirewall reconciles the firewall attached to the nodebalancer // -// This function does the follwing +// This function does the following // 1. If a firewallID annotation is present, it checks if the nodebalancer has a firewall attached, and if it matches the annotationID // a. If the IDs match, nothing to do here. -// b. If they don't match, then the nb is removed from the old FW, and its attached to the new firewall +// b. If they don't match, the nb is attached to the new firewall and removed from the old one. // 2. If a firewallACL annotation is present, // a. it checks if the nodebalancer has a firewall attached, if a fw exists, it updates rules // b. if a fw does not exist, it creates one @@ -365,13 +451,14 @@ func (l *loadbalancers) updateFWwithACL(ctx context.Context, service *v1.Service // a. AND if no firewalls are attached to the nodebalancer, nothing to do. // b. if the NB has ONE firewall attached, remove it from nb, and clean up if nothing else is attached to it // c. If there are more than one fw attached to it, then its a problem, return an err +// 4. If both these annotations are present, the firewallID takes precedence, and the ACL annotation is ignored. // IF a user creates a fw ID externally, and then switches to using a ACL, the CCM will take over the fw that's attached to the nodebalancer. func (l *loadbalancers) updateNodeBalancerFirewall(ctx context.Context, service *v1.Service, nb *linodego.NodeBalancer) error { // get the new firewall id from the annotation (if any). _, fwIDExists := service.GetAnnotations()[annLinodeCloudFirewallID] if fwIDExists { // If an ID exists, we ignore everything else and handle just that - return l.updateFWwithID(ctx, service, nb) + return l.updateFirewallwithID(ctx, service, nb) } // See if a acl exists @@ -385,29 +472,30 @@ func (l *loadbalancers) updateNodeBalancerFirewall(ctx context.Context, service if err != nil { return err } - switch len(firewalls) { - case 0: + + if len(firewalls) == 0 { return nil - case 1: - { - err := l.client.DeleteFirewallDevice(ctx, firewalls[0].ID, nb.ID) - if err != nil { - return err - } - // once we delete the device, we should see if there's anything attached to that firewall - devices, err := l.client.ListFirewallDevices(ctx, firewalls[0].ID, &linodego.ListOptions{}) - if err != nil { - return err - } - if len(devices) == 0 { - // nothing attached to it, clean it up - return l.client.DeleteFirewall(ctx, firewalls[0].ID) - } - // else let that firewall linger, don't mess with it. - } - default: + } + if len(firewalls) > 1 { return errTooManyFirewalls } + + err = l.client.DeleteFirewallDevice(ctx, firewalls[0].ID, nb.ID) + if err != nil { + return err + } + // once we delete the device, we should see if there's anything attached to that firewall + devices, err := l.client.ListFirewallDevices(ctx, firewalls[0].ID, &linodego.ListOptions{}) + if err != nil { + return err + } + + if len(devices) == 0 { + // nothing attached to it, clean it up + return l.client.DeleteFirewall(ctx, firewalls[0].ID) + } + // else let that firewall linger, don't mess with it. + return nil } @@ -672,6 +760,7 @@ func (l *loadbalancers) getLoadBalancerTags(_ context.Context, clusterName strin return tags } +// processACL takes the IPs, aclType, label etc and formats them into the passed linodego.FirewallCreateOptions pointer. func processACL(fwcreateOpts *linodego.FirewallCreateOptions, aclType, label, svcName, ports string, ips linodego.NetworkAddresses) { fwcreateOpts.Rules.Inbound = append(fwcreateOpts.Rules.Inbound, linodego.FirewallRule{ Action: aclType, @@ -704,9 +793,9 @@ func (l *loadbalancers) createFirewallOptsForSvc(label string, tags []string, sv Tags: tags, } - servicePorts := make([]string, len(svc.Spec.Ports)) - for idx, port := range svc.Spec.Ports { - servicePorts[idx] = strconv.Itoa(int(port.Port)) + var servicePorts []string + for _, port := range svc.Spec.Ports { + servicePorts = append(servicePorts, strconv.Itoa(int(port.Port))) } portsString := strings.Join(servicePorts[:], ",") diff --git a/cloud/linode/loadbalancers_test.go b/cloud/linode/loadbalancers_test.go index 44d0249d..2747f85a 100644 --- a/cloud/linode/loadbalancers_test.go +++ b/cloud/linode/loadbalancers_test.go @@ -1400,8 +1400,9 @@ func testUpdateLoadBalancerUpdateFirewallACL(t *testing.T, client *linodego.Clie svc.ObjectMeta.SetAnnotations(map[string]string{ annLinodeCloudFirewallACL: `{ - "denyList": { - "ipv4": ["2.2.2.2"] + "allowList": { + "ipv4": ["2.2.2.2"], + "ipv6": ["dead:beef::/128"] } }`, }) @@ -1425,13 +1426,21 @@ func testUpdateLoadBalancerUpdateFirewallACL(t *testing.T, client *linodego.Clie t.Fatalf("No attached firewalls found") } - if firewallsNew[0].Rules.InboundPolicy != "ACCEPT" { - t.Errorf("expected ACCEPT inbound policy, got %s", firewallsNew[0].Rules.InboundPolicy) - } - fwIPs = firewallsNew[0].Rules.Inbound[0].Addresses.IPv4 if fwIPs == nil { - t.Errorf("expected 2.2.2.2, got %v", fwIPs) + t.Errorf("expected non nil IPv4, got %v", fwIPs) + } + + if len(*fwIPs) != 1 { + t.Errorf("expected one IPv4, got %v", fwIPs) + } + + if firewallsNew[0].Rules.Inbound[0].Addresses.IPv6 == nil { + t.Errorf("expected non nil IPv6, got %v", firewallsNew[0].Rules.Inbound[0].Addresses.IPv6) + } + + if len(*firewallsNew[0].Rules.Inbound[0].Addresses.IPv6) != 1 { + t.Errorf("expected one IPv6, got %v", firewallsNew[0].Rules.Inbound[0].Addresses.IPv6) } } From 7a6fa692c21aaf0665ae591c7490c434f545ff79 Mon Sep 17 00:00:00 2001 From: Tarun Chinmai Sekar Date: Thu, 25 Jan 2024 17:12:20 +0000 Subject: [PATCH 15/19] lint --- cloud/linode/loadbalancers.go | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/cloud/linode/loadbalancers.go b/cloud/linode/loadbalancers.go index 22dc1ac1..24db63ad 100644 --- a/cloud/linode/loadbalancers.go +++ b/cloud/linode/loadbalancers.go @@ -8,11 +8,12 @@ import ( "net/http" "os" "reflect" - "slices" "strconv" "strings" "time" + "golang.org/x/exp/slices" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" @@ -315,14 +316,10 @@ func ipsChanged(ips *linodego.NetworkAddresses, rules []linodego.FirewallRule) b for _, rule := range rules { if rule.Addresses.IPv4 != nil { - for _, ip := range *rule.Addresses.IPv4 { - ruleIPv4s = append(ruleIPv4s, ip) - } + ruleIPv4s = append(ruleIPv4s, *rule.Addresses.IPv4...) } if rule.Addresses.IPv6 != nil { - for _, ip := range *rule.Addresses.IPv4 { - ruleIPv6s = append(ruleIPv6s, ip) - } + ruleIPv6s = append(ruleIPv6s, *rule.Addresses.IPv6...) } } @@ -793,9 +790,9 @@ func (l *loadbalancers) createFirewallOptsForSvc(label string, tags []string, sv Tags: tags, } - var servicePorts []string - for _, port := range svc.Spec.Ports { - servicePorts = append(servicePorts, strconv.Itoa(int(port.Port))) + servicePorts := make([]string, len(svc.Spec.Ports)) + for idx, port := range svc.Spec.Ports { + servicePorts[idx] = strconv.Itoa(int(port.Port)) } portsString := strings.Join(servicePorts[:], ",") From 74d75ea18bf33a57fd2463f1dced3729e01c66dc Mon Sep 17 00:00:00 2001 From: Tarun Chinmai Sekar Date: Thu, 25 Jan 2024 17:13:30 +0000 Subject: [PATCH 16/19] add x/exp/slices --- go.mod | 15 ++++++++------- go.sum | 27 ++++++++++++++++----------- 2 files changed, 24 insertions(+), 18 deletions(-) diff --git a/go.mod b/go.mod index 214412d8..96c38329 100644 --- a/go.mod +++ b/go.mod @@ -9,6 +9,7 @@ require ( github.com/linode/linodego v1.26.0 github.com/spf13/pflag v1.0.5 github.com/stretchr/testify v1.8.4 + golang.org/x/exp v0.0.0-20240119083558-1b970713d09a k8s.io/api v0.21.0 k8s.io/apimachinery v0.21.0 k8s.io/client-go v0.21.0 @@ -70,17 +71,17 @@ require ( go.uber.org/multierr v1.3.0 // indirect go.uber.org/tools v0.0.0-20190618225709-2cfd321de3ee // indirect go.uber.org/zap v1.13.0 // indirect - golang.org/x/crypto v0.16.0 // indirect + golang.org/x/crypto v0.18.0 // indirect golang.org/x/lint v0.0.0-20200302205851-738671d3881b // indirect - golang.org/x/mod v0.8.0 // indirect - golang.org/x/net v0.19.0 // indirect + golang.org/x/mod v0.14.0 // indirect + golang.org/x/net v0.20.0 // indirect golang.org/x/oauth2 v0.0.0-20200107190931-bf48bf16ab8d // indirect - golang.org/x/sync v0.1.0 // indirect - golang.org/x/sys v0.15.0 // indirect - golang.org/x/term v0.15.0 // indirect + golang.org/x/sync v0.6.0 // indirect + golang.org/x/sys v0.16.0 // indirect + golang.org/x/term v0.16.0 // indirect golang.org/x/text v0.14.0 // indirect golang.org/x/time v0.0.0-20211116232009-f0f3c7e86c11 // indirect - golang.org/x/tools v0.6.0 // indirect + golang.org/x/tools v0.17.0 // indirect google.golang.org/appengine v1.6.5 // indirect google.golang.org/genproto v0.0.0-20201110150050-8816d57aaa9a // indirect google.golang.org/grpc v1.27.1 // indirect diff --git a/go.sum b/go.sum index 8ef40708..ef6c9a1d 100644 --- a/go.sum +++ b/go.sum @@ -526,8 +526,8 @@ golang.org/x/crypto v0.0.0-20201002170205-7f63de1d35b0/go.mod h1:LzIPMQfyMNhhGPh golang.org/x/crypto v0.0.0-20210220033148-5ea612d1eb83/go.mod h1:jdWPYTVW3xRLrWPugEBEK3UY2ZEsg3UU495nc5E+M+I= golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= golang.org/x/crypto v0.13.0/go.mod h1:y6Z2r+Rw4iayiXXAIxJIDAJ1zMW4yaTpebo8fPOliYc= -golang.org/x/crypto v0.16.0 h1:mMMrFzRSCF0GvB7Ne27XVtVAaXLrPmgPC7/v0tkwHaY= -golang.org/x/crypto v0.16.0/go.mod h1:gCAAfMLgwOJRpTjQ2zCCt2OcSfYMTeZVSRtQlPC7Nq4= +golang.org/x/crypto v0.18.0 h1:PGVlW0xEltQnzFZ55hkuX5+KLyrMYhHld1YHO4AKcdc= +golang.org/x/crypto v0.18.0/go.mod h1:R0j02AL6hcrfOiy9T4ZYp/rcWeMxM3L6QYxlOuEG1mg= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20190306152737-a1d7652674e8/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20190510132918-efd6b22b2522/go.mod h1:ZjyILWgesfNpC6sMxTJOJm9Kp84zZh5NQWvqDGG3Qr8= @@ -538,6 +538,8 @@ golang.org/x/exp v0.0.0-20191227195350-da58074b4299/go.mod h1:2RIsYlXP63K8oxa1u0 golang.org/x/exp v0.0.0-20200119233911-0405dc783f0a/go.mod h1:2RIsYlXP63K8oxa1u096TMicItID8zy7Y6sNkU49FU4= golang.org/x/exp v0.0.0-20200207192155-f17229e696bd/go.mod h1:J/WKrq2StrnmMY6+EHIKF9dgMWnmCNThgcyBT1FY9mM= golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6/go.mod h1:3jZMyOhIsHpP37uCMkUooju7aAi5cS1Q23tOzKc+0MU= +golang.org/x/exp v0.0.0-20240119083558-1b970713d09a h1:Q8/wZp0KX97QFTc2ywcOE0YRjZPVIx+MXInMzdvQqcA= +golang.org/x/exp v0.0.0-20240119083558-1b970713d09a/go.mod h1:idGWGoKP1toJGkd5/ig9ZLuPcZBC3ewk7SzmH0uou08= golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js= golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0= golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= @@ -561,8 +563,9 @@ golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= -golang.org/x/mod v0.8.0 h1:LUYupSeNrTNCGzR/hVBk2NHZO4hXcVaW1k4Qx7rjPx8= golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= +golang.org/x/mod v0.14.0 h1:dGoOF9QVLYng8IHTm7BAyWqCqSheQ5pYWGhzW00YJr0= +golang.org/x/mod v0.14.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= @@ -598,8 +601,8 @@ golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug golang.org/x/net v0.6.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= golang.org/x/net v0.10.0/go.mod h1:0qNGK6F8kojg2nk9dLZ2mShWaEBan6FAoqfSigmmuDg= golang.org/x/net v0.15.0/go.mod h1:idbUs1IY1+zTqbi8yxTbhexhEEk5ur9LInksu6HrEpk= -golang.org/x/net v0.19.0 h1:zTwKpTd2XuCqf8huc7Fo2iSy+4RHPd10s4KzeTnVr1c= -golang.org/x/net v0.19.0/go.mod h1:CfAk/cbD4CthTvqiEl8NpboMuiuOYsAr/7NOjZJtv1U= +golang.org/x/net v0.20.0 h1:aCL9BSgETF1k+blQaYUBx9hJ9LOGP3gAVemcZlf1Kpo= +golang.org/x/net v0.20.0/go.mod h1:z8BVo6PvndSri0LbOE3hAn0apkU+1YvI6E70E9jsnvY= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= @@ -615,8 +618,9 @@ golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJ golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.1.0 h1:wsuoTGHzEhffawBOhz5CYhcrV4IdKZbEyZjBMuTp12o= golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.6.0 h1:5BMeUDZ7vkXGfEr1x9B4bRcTH4lpkTkpdh0T/J+qjbQ= +golang.org/x/sync v0.6.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= golang.org/x/sys v0.0.0-20180823144017-11551d06cbcc/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20180905080454-ebe1bf3edb33/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= @@ -664,8 +668,8 @@ golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.12.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.15.0 h1:h48lPFYpsTvQJZF4EKyI4aLHaev3CxivZmv7yZig9pc= -golang.org/x/sys v0.15.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/sys v0.16.0 h1:xWw16ngr6ZMtmxDyKyIgsE93KNKz5HKmMa3b8ALHidU= +golang.org/x/sys v0.16.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210220032956-6a3ed077a48d/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= @@ -673,8 +677,8 @@ golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuX golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k= golang.org/x/term v0.8.0/go.mod h1:xPskH00ivmX89bAKVGSKKtLOWNx2+17Eiy94tnKShWo= golang.org/x/term v0.12.0/go.mod h1:owVbMEjm3cBLCHdkQu9b1opXd4ETQWc3BhuQGKgXgvU= -golang.org/x/term v0.15.0 h1:y/Oo/a/q3IXu26lQgl04j/gjuBDOBlx7X6Om1j2CPW4= -golang.org/x/term v0.15.0/go.mod h1:BDl952bC7+uMoWR75FIrCDx79TPU9oHkTZ9yRbYOrX0= +golang.org/x/term v0.16.0 h1:m+B6fahuftsE9qjo0VWp2FW0mB3MTJvR0BaMQrq0pmE= +golang.org/x/term v0.16.0/go.mod h1:yn7UURbUtPyrVJPGPq404EukNFxcm/foM+bV/bfcDsY= golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= @@ -737,8 +741,9 @@ golang.org/x/tools v0.0.0-20200619180055-7c47624df98f/go.mod h1:EkVYQZoAsY45+roY golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= golang.org/x/tools v0.1.1/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= -golang.org/x/tools v0.6.0 h1:BOw41kyTf3PuCW1pVQf8+Cyg8pMlkYB1oo9iJ6D/lKM= golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU= +golang.org/x/tools v0.17.0 h1:FvmRgNOcs3kOa+T20R1uhfP9F6HgG2mfxDv1vrx1Htc= +golang.org/x/tools v0.17.0/go.mod h1:xsh6VxdV005rRVaS6SSAf9oiAqljS7UZUacMZ8Bnsps= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= From 98b5de96d185520a381aeba0c0b9a65332a933ed Mon Sep 17 00:00:00 2001 From: Tarun Chinmai Sekar Date: Fri, 26 Jan 2024 18:32:06 +0000 Subject: [PATCH 17/19] fixup --- cloud/linode/loadbalancers.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/cloud/linode/loadbalancers.go b/cloud/linode/loadbalancers.go index 24db63ad..be09dfb2 100644 --- a/cloud/linode/loadbalancers.go +++ b/cloud/linode/loadbalancers.go @@ -789,10 +789,9 @@ func (l *loadbalancers) createFirewallOptsForSvc(label string, tags []string, sv Label: label, Tags: tags, } - - servicePorts := make([]string, len(svc.Spec.Ports)) - for idx, port := range svc.Spec.Ports { - servicePorts[idx] = strconv.Itoa(int(port.Port)) + servicePorts := make([]string, 0, len(svc.Spec.Ports)) + for _, port := range svc.Spec.Ports { + servicePorts = append(servicePorts, strconv.Itoa(int(port.Port))) } portsString := strings.Join(servicePorts[:], ",") From 253b4f1a1b1c12f2ec02a82930254e049c9e16f7 Mon Sep 17 00:00:00 2001 From: Tarun Chinmai Sekar Date: Sat, 27 Jan 2024 18:43:53 +0000 Subject: [PATCH 18/19] fixes --- cloud/linode/loadbalancers.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cloud/linode/loadbalancers.go b/cloud/linode/loadbalancers.go index be09dfb2..af19489b 100644 --- a/cloud/linode/loadbalancers.go +++ b/cloud/linode/loadbalancers.go @@ -27,9 +27,9 @@ import ( ) var ( - errNoNodesAvailable = errors.New("no nodes available for nodebalancer") + errNoNodesAvailable = errors.New("No nodes available for nodebalancer") errInvalidFWConfig = errors.New("Specify either an allowList or a denyList for a firewall") - errTooManyFirewalls = errors.New("Too many firewalls attached to a nodebalancer") + errTooManyFirewalls = errors.New("Too many firewalls attached to a nodebalancer") ) type lbNotFoundError struct { From 8476f120905120338205ec47e79193d93fdd2a8c Mon Sep 17 00:00:00 2001 From: Tarun Chinmai Sekar Date: Mon, 29 Jan 2024 16:59:30 +0000 Subject: [PATCH 19/19] Review comments --- cloud/linode/loadbalancers.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/cloud/linode/loadbalancers.go b/cloud/linode/loadbalancers.go index af19489b..7270ed8a 100644 --- a/cloud/linode/loadbalancers.go +++ b/cloud/linode/loadbalancers.go @@ -231,8 +231,8 @@ func (l *loadbalancers) EnsureLoadBalancer(ctx context.Context, clusterName stri return lbStatus, nil } -// getNodeBalancerDeviceId gets the deviceID of the nodeBalancer that is attached to the firewall. -func (l *loadbalancers) getNodeBalancerDeviceId(ctx context.Context, firewallID, nbID int) (int, bool, error) { +// getNodeBalancerDeviceID gets the deviceID of the nodeBalancer that is attached to the firewall. +func (l *loadbalancers) getNodeBalancerDeviceID(ctx context.Context, firewallID, nbID int) (int, bool, error) { devices, err := l.client.ListFirewallDevices(ctx, firewallID, &linodego.ListOptions{}) if err != nil { return 0, false, err @@ -271,6 +271,7 @@ func (l *loadbalancers) updateFirewallwithID(ctx context.Context, service *v1.Se return err } if len(firewalls) > 1 { + klog.Errorf("Found more than one firewall attached to nodebalancer: %d, firewall IDs: %v", nb.ID, firewalls) return errTooManyFirewalls } @@ -292,7 +293,7 @@ func (l *loadbalancers) updateFirewallwithID(ctx context.Context, service *v1.Se } // remove the existing firewall if it exists if existingFirewallID != 0 { - deviceID, deviceExists, err := l.getNodeBalancerDeviceId(ctx, existingFirewallID, nb.ID) + deviceID, deviceExists, err := l.getNodeBalancerDeviceID(ctx, existingFirewallID, nb.ID) if err != nil { return err } @@ -430,6 +431,7 @@ func (l *loadbalancers) updateFWwithACL(ctx context.Context, service *v1.Service } } default: + klog.Errorf("Found more than one firewall attached to nodebalancer: %d, firewall IDs: %v", nb.ID, firewalls) return errTooManyFirewalls } return nil @@ -474,6 +476,7 @@ func (l *loadbalancers) updateNodeBalancerFirewall(ctx context.Context, service return nil } if len(firewalls) > 1 { + klog.Errorf("Found more than one firewall attached to nodebalancer: %d, firewall IDs: %v", nb.ID, firewalls) return errTooManyFirewalls }