From 91feb1f750de70a2008b81f3bf0dda41aa4be13f Mon Sep 17 00:00:00 2001 From: Gerrit Date: Tue, 10 Sep 2024 13:29:38 +0200 Subject: [PATCH] Update gardener infrastructure egress cidrs for ACL extension. (#61) --- .../deployment/infrastructure_status.go | 163 ++++++ .../deployment/infrastructure_status_test.go | 522 ++++++++++++++++++ controllers/deployment/reconcile.go | 42 +- controllers/deployment/status.go | 11 +- 4 files changed, 722 insertions(+), 16 deletions(-) create mode 100644 controllers/deployment/infrastructure_status.go create mode 100644 controllers/deployment/infrastructure_status_test.go diff --git a/controllers/deployment/infrastructure_status.go b/controllers/deployment/infrastructure_status.go new file mode 100644 index 0000000..1caa62b --- /dev/null +++ b/controllers/deployment/infrastructure_status.go @@ -0,0 +1,163 @@ +package deployment + +import ( + "encoding/json" + "fmt" + "net/netip" + "slices" + "strings" + + v2 "github.com/metal-stack/firewall-controller-manager/api/v2" + "github.com/metal-stack/firewall-controller-manager/controllers" + "github.com/metal-stack/metal-lib/pkg/pointer" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +func (c *controller) updateInfrastructureStatus(r *controllers.Ctx[*v2.FirewallDeployment], infrastructureName string, ownedFirewalls []*v2.Firewall) error { + infraObj := &unstructured.Unstructured{} + + infraObj.SetGroupVersionKind(schema.GroupVersionKind{ + Group: "extensions.gardener.cloud", + Kind: "Infrastructure", + Version: "v1alpha1", + }) + + err := c.c.GetSeedClient().Get(r.Ctx, client.ObjectKey{ + Namespace: c.c.GetSeedNamespace(), + Name: infrastructureName, + }, infraObj) + if err != nil { + if apierrors.IsNotFound(err) { + return nil + } + return err + } + + type infrastructure struct { + Spec struct { + ProviderConfig struct { + Firewall struct { + EgressRules []struct { + IPs []string `json:"ips"` + } `json:"egressRules"` + } `json:"firewall"` + } `json:"providerConfig"` + } `json:"spec"` + Status struct { + EgressCIDRs []string `json:"egressCIDRs"` + } `json:"status"` + } + + infraRaw, err := json.Marshal(infraObj) + if err != nil { + return fmt.Errorf("unable to convert gardener infrastructure object: %w", err) + } + + var typedInfra infrastructure + err = json.Unmarshal(infraRaw, &typedInfra) + if err != nil { + return fmt.Errorf("unable to convert gardener infrastructure object: %w", err) + } + + var egressCIDRs []string + + for _, fw := range ownedFirewalls { + for _, network := range fw.Status.FirewallNetworks { + if pointer.SafeDeref(network.NetworkType) != "external" { + continue + } + + for _, ip := range network.IPs { + parsed, err := netip.ParseAddr(ip) + if err != nil { + continue + } + + egressCIDRs = append(egressCIDRs, fmt.Sprintf("%s/%d", ip, parsed.BitLen())) + } + } + } + + for _, rule := range typedInfra.Spec.ProviderConfig.Firewall.EgressRules { + for _, ip := range rule.IPs { + parsed, err := netip.ParseAddr(ip) + if err != nil { + continue + } + + egressCIDRs = append(egressCIDRs, fmt.Sprintf("%s/%d", ip, parsed.BitLen())) + } + } + + slices.Sort(egressCIDRs) + slices.Sort(typedInfra.Status.EgressCIDRs) + + // check if an update is required or not + if slices.Equal(egressCIDRs, typedInfra.Status.EgressCIDRs) { + c.log.Info("found gardener infrastructure resource, egress cidrs already up-to-date", "infrastructure-name", infraObj.GetName(), "egress-cidrs", egressCIDRs) + return nil + } + + infraStatusPatch := map[string]any{ + "status": map[string]any{ + "egressCIDRs": egressCIDRs, + }, + } + + jsonPatch, err := json.Marshal(infraStatusPatch) + if err != nil { + return fmt.Errorf("unable to marshal infrastructure status patch: %w", err) + } + + err = c.c.GetSeedClient().Status().Patch(r.Ctx, infraObj, client.RawPatch(types.MergePatchType, jsonPatch)) + if err != nil { + return fmt.Errorf("error patching infrastructure status egress cidrs field: %w", err) + } + + c.log.Info("found gardener infrastructure resource and patched egress cidrs for acl extension", "infrastructure-name", infraObj.GetName(), "egress-cidrs", egressCIDRs) + + aclObj := &unstructured.Unstructured{} + + aclObj.SetGroupVersionKind(schema.GroupVersionKind{ + Group: "extensions.gardener.cloud", + Kind: "Extension", + Version: "v1alpha1", + }) + + err = c.c.GetSeedClient().Get(r.Ctx, client.ObjectKey{ + Namespace: c.c.GetSeedNamespace(), + Name: "acl", + }, aclObj) + if err != nil { + if apierrors.IsNotFound(err) { + return nil + } + return err + } + + err = v2.AddAnnotation(r.Ctx, c.c.GetSeedClient(), aclObj, "gardener.cloud/operation", "reconcile") + if err != nil { + return fmt.Errorf("error annotating acl extension with reconcile operation: %w", err) + } + + c.log.Info("added reconcile annotation to gardener acl extension object") + + return nil +} + +func extractInfrastructureNameFromSeedNamespace(namespace string) (string, bool) { + if !strings.HasPrefix(namespace, "shoot--") { + return "", false + } + + parts := strings.Split(namespace, "--") + if len(parts) < 3 { + return "", false + } + + return strings.Join(parts[2:], "--"), true +} diff --git a/controllers/deployment/infrastructure_status_test.go b/controllers/deployment/infrastructure_status_test.go new file mode 100644 index 0000000..96fa1cb --- /dev/null +++ b/controllers/deployment/infrastructure_status_test.go @@ -0,0 +1,522 @@ +package deployment + +import ( + "context" + "testing" + + "github.com/go-logr/logr/testr" + "github.com/google/go-cmp/cmp" + v2 "github.com/metal-stack/firewall-controller-manager/api/v2" + "github.com/metal-stack/firewall-controller-manager/api/v2/config" + "github.com/metal-stack/firewall-controller-manager/controllers" + "github.com/metal-stack/metal-lib/pkg/pointer" + "github.com/metal-stack/metal-lib/pkg/testcommon" + "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func Test_controller_updateInfrastructureStatus(t *testing.T) { + scheme := runtime.NewScheme() + ctx := context.Background() + log := testr.New(t) + + testNamespace := "shoot--abcdef--mycluster1" + + tests := []struct { + name string + objs []client.Object + ownedFirewalls []*v2.Firewall + want []client.Object + wantErr error + }{ + { + name: "no infrastructure present", + objs: nil, + wantErr: nil, + }, + { + name: "infrastructure is present, egress cidrs were not yet set (acl extension gets annotated)", + objs: []client.Object{ + &unstructured.Unstructured{ + Object: map[string]any{ + "apiVersion": "extensions.gardener.cloud/v1alpha1", + "kind": "Infrastructure", + "metadata": map[string]any{ + "name": "mycluster1", + "namespace": testNamespace, + "resourceVersion": "999", + }, + "spec": map[string]any{ + "providerConfig": map[string]any{ + "apiVersion": "metal.provider.extensions.gardener.cloud/v1alpha1", + "firewall": map[string]any{ + "controllerVersion": "auto", + }, + }, + }, + "status": map[string]any{ + "phase": "foo", + }, + }, + }, + &unstructured.Unstructured{ + Object: map[string]any{ + "apiVersion": "extensions.gardener.cloud/v1alpha1", + "kind": "Extension", + "metadata": map[string]any{ + "name": "acl", + "namespace": testNamespace, + "resourceVersion": "999", + }, + }, + }, + }, + ownedFirewalls: []*v2.Firewall{ + { + Status: v2.FirewallStatus{ + FirewallNetworks: []v2.FirewallNetwork{ + { + NetworkType: pointer.Pointer("external"), + IPs: []string{"1.1.1.1"}, + }, + { + NetworkType: pointer.Pointer("underlay"), + IPs: []string{"10.8.0.4"}, + }, + }, + }, + }, + }, + want: []client.Object{ + &unstructured.Unstructured{ + Object: map[string]any{ + "apiVersion": "extensions.gardener.cloud/v1alpha1", + "kind": "Infrastructure", + "metadata": map[string]any{ + "name": "mycluster1", + "namespace": testNamespace, + "resourceVersion": "1000", + }, + "spec": map[string]any{ + "providerConfig": map[string]any{ + "apiVersion": "metal.provider.extensions.gardener.cloud/v1alpha1", + "firewall": map[string]any{ + "controllerVersion": "auto", + }, + }, + }, + "status": map[string]any{ + "phase": "foo", + "egressCIDRs": []any{"1.1.1.1/32"}, + }, + }, + }, + &unstructured.Unstructured{ + Object: map[string]any{ + "apiVersion": "extensions.gardener.cloud/v1alpha1", + "kind": "Extension", + "metadata": map[string]any{ + "name": "acl", + "namespace": testNamespace, + "resourceVersion": "1000", + "annotations": map[string]any{ + "gardener.cloud/operation": "reconcile", + }, + }, + "status": nil, + }, + }, + }, + wantErr: nil, + }, + { + name: "infrastructure is present, egress cidrs have already been set but not up-to-date", + objs: []client.Object{ + &unstructured.Unstructured{ + Object: map[string]any{ + "apiVersion": "extensions.gardener.cloud/v1alpha1", + "kind": "Infrastructure", + "metadata": map[string]any{ + "name": "mycluster1", + "namespace": testNamespace, + "resourceVersion": "999", + }, + "spec": map[string]any{ + "providerConfig": map[string]any{ + "apiVersion": "metal.provider.extensions.gardener.cloud/v1alpha1", + "firewall": map[string]any{ + "controllerVersion": "auto", + }, + }, + }, + "status": map[string]any{ + "phase": "foo", + "egressCIDRs": []any{"1.2.3.4/32", "2.3.4.5/32"}, + }, + }, + }, + }, + ownedFirewalls: []*v2.Firewall{ + { + Status: v2.FirewallStatus{ + FirewallNetworks: []v2.FirewallNetwork{ + { + NetworkType: pointer.Pointer("external"), + IPs: []string{"1.1.1.1"}, + }, + { + NetworkType: pointer.Pointer("underlay"), + IPs: []string{"10.8.0.4"}, + }, + }, + }, + }, + }, + want: []client.Object{ + &unstructured.Unstructured{ + Object: map[string]any{ + "apiVersion": "extensions.gardener.cloud/v1alpha1", + "kind": "Infrastructure", + "metadata": map[string]any{ + "name": "mycluster1", + "namespace": testNamespace, + "resourceVersion": "1000", + }, + "spec": map[string]any{ + "providerConfig": map[string]any{ + "apiVersion": "metal.provider.extensions.gardener.cloud/v1alpha1", + "firewall": map[string]any{ + "controllerVersion": "auto", + }, + }, + }, + "status": map[string]any{ + "phase": "foo", + "egressCIDRs": []any{"1.1.1.1/32"}, + }, + }, + }, + }, + wantErr: nil, + }, + { + name: "infrastructure is present, egress cidrs have already been set and are up-to-date", + objs: []client.Object{ + &unstructured.Unstructured{ + Object: map[string]any{ + "apiVersion": "extensions.gardener.cloud/v1alpha1", + "kind": "Infrastructure", + "metadata": map[string]any{ + "name": "mycluster1", + "namespace": testNamespace, + "resourceVersion": "999", + }, + "spec": map[string]any{ + "providerConfig": map[string]any{ + "apiVersion": "metal.provider.extensions.gardener.cloud/v1alpha1", + "firewall": map[string]any{ + "controllerVersion": "auto", + }, + }, + }, + "status": map[string]any{ + "phase": "foo", + "egressCIDRs": []any{"1.1.1.1/32"}, + }, + }, + }, + }, + ownedFirewalls: []*v2.Firewall{ + { + Status: v2.FirewallStatus{ + FirewallNetworks: []v2.FirewallNetwork{ + { + NetworkType: pointer.Pointer("external"), + IPs: []string{"1.1.1.1"}, + }, + { + NetworkType: pointer.Pointer("underlay"), + IPs: []string{"10.8.0.4"}, + }, + }, + }, + }, + }, + want: []client.Object{ + &unstructured.Unstructured{ + Object: map[string]any{ + "apiVersion": "extensions.gardener.cloud/v1alpha1", + "kind": "Infrastructure", + "metadata": map[string]any{ + "name": "mycluster1", + "namespace": testNamespace, + "resourceVersion": "999", + }, + "spec": map[string]any{ + "providerConfig": map[string]any{ + "apiVersion": "metal.provider.extensions.gardener.cloud/v1alpha1", + "firewall": map[string]any{ + "controllerVersion": "auto", + }, + }, + }, + "status": map[string]any{ + "phase": "foo", + "egressCIDRs": []any{"1.1.1.1/32"}, + }, + }, + }, + }, + wantErr: nil, + }, + { + name: "infrastructure has egress rules", + objs: []client.Object{ + &unstructured.Unstructured{ + Object: map[string]any{ + "apiVersion": "extensions.gardener.cloud/v1alpha1", + "kind": "Infrastructure", + "metadata": map[string]any{ + "name": "mycluster1", + "namespace": testNamespace, + "resourceVersion": "999", + }, + "spec": map[string]any{ + "providerConfig": map[string]any{ + "apiVersion": "metal.provider.extensions.gardener.cloud/v1alpha1", + "firewall": map[string]any{ + "controllerVersion": "auto", + "egressRules": []any{ + map[string]any{ + "ips": []any{"3.4.5.6"}, + }, + }, + }, + }, + }, + "status": map[string]any{ + "phase": "foo", + "egressCIDRs": []any{"1.1.1.1/32"}, + }, + }, + }, + }, + ownedFirewalls: []*v2.Firewall{ + { + Status: v2.FirewallStatus{ + FirewallNetworks: []v2.FirewallNetwork{ + { + NetworkType: pointer.Pointer("external"), + IPs: []string{"1.1.1.1"}, + }, + { + NetworkType: pointer.Pointer("underlay"), + IPs: []string{"10.8.0.4"}, + }, + }, + }, + }, + }, + want: []client.Object{ + &unstructured.Unstructured{ + Object: map[string]any{ + "apiVersion": "extensions.gardener.cloud/v1alpha1", + "kind": "Infrastructure", + "metadata": map[string]any{ + "name": "mycluster1", + "namespace": testNamespace, + "resourceVersion": "1000", + }, + "spec": map[string]any{ + "providerConfig": map[string]any{ + "apiVersion": "metal.provider.extensions.gardener.cloud/v1alpha1", + "firewall": map[string]any{ + "controllerVersion": "auto", + "egressRules": []any{ + map[string]any{ + "ips": []any{"3.4.5.6"}, + }, + }, + }, + }, + }, + "status": map[string]any{ + "phase": "foo", + "egressCIDRs": []any{"1.1.1.1/32", "3.4.5.6/32"}, + }, + }, + }, + }, + wantErr: nil, + }, + { + name: "skip update on different order of ip elements in slice", + objs: []client.Object{ + &unstructured.Unstructured{ + Object: map[string]any{ + "apiVersion": "extensions.gardener.cloud/v1alpha1", + "kind": "Infrastructure", + "metadata": map[string]any{ + "name": "mycluster1", + "namespace": testNamespace, + "resourceVersion": "999", + }, + "spec": map[string]any{ + "providerConfig": map[string]any{ + "apiVersion": "metal.provider.extensions.gardener.cloud/v1alpha1", + "firewall": map[string]any{ + "controllerVersion": "auto", + }, + }, + }, + "status": map[string]any{ + "phase": "foo", + "egressCIDRs": []any{"1.1.1.2/32", "1.1.1.1/32"}, + }, + }, + }, + }, + ownedFirewalls: []*v2.Firewall{ + { + Status: v2.FirewallStatus{ + FirewallNetworks: []v2.FirewallNetwork{ + { + NetworkType: pointer.Pointer("external"), + IPs: []string{"1.1.1.1"}, + }, + { + NetworkType: pointer.Pointer("underlay"), + IPs: []string{"10.8.0.4"}, + }, + }, + }, + }, + { + Status: v2.FirewallStatus{ + FirewallNetworks: []v2.FirewallNetwork{ + { + NetworkType: pointer.Pointer("external"), + IPs: []string{"1.1.1.2"}, + }, + { + NetworkType: pointer.Pointer("underlay"), + IPs: []string{"10.8.0.5"}, + }, + }, + }, + }, + }, + want: []client.Object{ + &unstructured.Unstructured{ + Object: map[string]any{ + "apiVersion": "extensions.gardener.cloud/v1alpha1", + "kind": "Infrastructure", + "metadata": map[string]any{ + "name": "mycluster1", + "namespace": testNamespace, + "resourceVersion": "999", + }, + "spec": map[string]any{ + "providerConfig": map[string]any{ + "apiVersion": "metal.provider.extensions.gardener.cloud/v1alpha1", + "firewall": map[string]any{ + "controllerVersion": "auto", + }, + }, + }, + "status": map[string]any{ + "phase": "foo", + "egressCIDRs": []any{"1.1.1.2/32", "1.1.1.1/32"}, + }, + }, + }, + }, + wantErr: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(tt.objs...).WithStatusSubresource(tt.objs...).Build() + + cc, err := config.New(&config.NewControllerConfig{ + SeedClient: c, + SeedNamespace: testNamespace, + SkipValidation: true, + }) + require.NoError(t, err) + + ctrl := &controller{ + log: log, + c: cc, + } + + err = ctrl.updateInfrastructureStatus(&controllers.Ctx[*v2.FirewallDeployment]{ + Ctx: ctx, + Log: log, + }, "mycluster1", tt.ownedFirewalls) + if diff := cmp.Diff(tt.wantErr, err, testcommon.ErrorStringComparer()); diff != "" { + t.Errorf("error diff (+got -want):\n %s", diff) + } + + for _, want := range tt.want { + u := unstructured.Unstructured{} + u.SetGroupVersionKind(want.GetObjectKind().GroupVersionKind()) + + err = c.Get(ctx, client.ObjectKeyFromObject(want), &u) + require.NoError(t, err) + + if diff := cmp.Diff(want, &u); diff != "" { + t.Errorf("diff (+got -want):\n %s", diff) + } + } + }) + } +} + +func Test_extractInfrastructureNameFromSeedNamespace(t *testing.T) { + tests := []struct { + name string + namespace string + want string + wantBool bool + }{ + { + name: "default namespace not considered", + namespace: "default", + want: "", + wantBool: false, + }, + { + name: "correctly extract from gardener namespace scheme", + namespace: "shoot--abcdef--mycluster1", + want: "mycluster1", + wantBool: true, + }, + { + name: "incorrect namespace scheme #1", + namespace: "shoot--abcdef", + want: "", + wantBool: false, + }, + { + name: "another double-hyphen in cluster name", + namespace: "shoot--abcdef--myclust--er1", + want: "myclust--er1", + wantBool: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, gotBool := extractInfrastructureNameFromSeedNamespace(tt.namespace) + if diff := cmp.Diff(got, tt.want); diff != "" { + t.Errorf("diff (+got -want):\n %s", diff) + } + if diff := cmp.Diff(gotBool, tt.wantBool); diff != "" { + t.Errorf("diff (+got -want):\n %s", diff) + } + }) + } +} diff --git a/controllers/deployment/reconcile.go b/controllers/deployment/reconcile.go index c427791..2b5625f 100644 --- a/controllers/deployment/reconcile.go +++ b/controllers/deployment/reconcile.go @@ -43,22 +43,32 @@ func (c *controller) Reconcile(r *controllers.Ctx[*v2.FirewallDeployment]) error return nil } + var reconcileErr error switch s := r.Target.Spec.Strategy; s { case v2.StrategyRecreate: - err = c.recreateStrategy(r, ownedSets, latestSet) + reconcileErr = c.recreateStrategy(r, ownedSets, latestSet) case v2.StrategyRollingUpdate: - err = c.rollingUpdateStrategy(r, ownedSets, latestSet) + reconcileErr = c.rollingUpdateStrategy(r, ownedSets, latestSet) default: - err = fmt.Errorf("unknown deployment strategy: %s", s) + reconcileErr = fmt.Errorf("unknown deployment strategy: %s", s) } - statusErr := c.setStatus(r) + // refetch sets after possible creation + // we want to update the set status before a possible return due to reconciliation having failed + ownedSets, _, err = controllers.GetOwnedResources(r.Ctx, c.c.GetSeedClient(), nil, r.Target, &v2.FirewallSetList{}, func(fsl *v2.FirewallSetList) []*v2.FirewallSet { + return fsl.GetItems() + }) + if err != nil { + return fmt.Errorf("unable to get owned sets: %w", err) + } + err = c.setStatus(r, ownedSets) if err != nil { return err } - if statusErr != nil { - return err + + if reconcileErr != nil { + return reconcileErr } // we are done with the update, give the set the shortest distance if this is not already the case @@ -73,6 +83,26 @@ func (c *controller) Reconcile(r *controllers.Ctx[*v2.FirewallDeployment]) error r.Log.Info("swapped latest set to shortest distance", "distance", v2.FirewallShortestDistance) } + infrastructureName, ok := extractInfrastructureNameFromSeedNamespace(c.c.GetSeedNamespace()) + if ok { + var ownedFirewalls []*v2.Firewall + for _, set := range ownedSets { + fws, _, err := controllers.GetOwnedResources(r.Ctx, c.c.GetSeedClient(), nil, set, &v2.FirewallList{}, func(fl *v2.FirewallList) []*v2.Firewall { + return fl.GetItems() + }) + if err != nil { + return fmt.Errorf("unable to get owned firewalls: %w", err) + } + + ownedFirewalls = append(ownedFirewalls, fws...) + } + + err = c.updateInfrastructureStatus(r, infrastructureName, ownedFirewalls) + if err != nil { + return err + } + } + return nil } diff --git a/controllers/deployment/status.go b/controllers/deployment/status.go index 02b78eb..abe1999 100644 --- a/controllers/deployment/status.go +++ b/controllers/deployment/status.go @@ -1,20 +1,11 @@ package deployment import ( - "fmt" - v2 "github.com/metal-stack/firewall-controller-manager/api/v2" "github.com/metal-stack/firewall-controller-manager/controllers" ) -func (c *controller) setStatus(r *controllers.Ctx[*v2.FirewallDeployment]) error { - ownedSets, _, err := controllers.GetOwnedResources(r.Ctx, c.c.GetSeedClient(), nil, r.Target, &v2.FirewallSetList{}, func(fsl *v2.FirewallSetList) []*v2.FirewallSet { - return fsl.GetItems() - }) - if err != nil { - return fmt.Errorf("unable to get owned sets: %w", err) - } - +func (c *controller) setStatus(r *controllers.Ctx[*v2.FirewallDeployment], ownedSets []*v2.FirewallSet) error { latestSet, err := controllers.MaxRevisionOf(ownedSets) if err != nil { return err