From 687218a61b30397e125c18eaf279792a670c62b5 Mon Sep 17 00:00:00 2001 From: Lukas Frank Date: Tue, 6 Dec 2022 15:08:29 +0100 Subject: [PATCH 1/2] Implemented `NatGateway` --- api/v1alpha1/publicip_types.go | 10 + apinetlet/api/v1alpha1/common_types.go | 4 + .../controllers/controllers_suite_test.go | 6 + apinetlet/controllers/helper.go | 30 ++ .../controllers/natgateway_controller.go | 275 ++++++++++++++++++ .../controllers/natgateway_controller_test.go | 190 ++++++++++++ apinetlet/controllers/virtualip_controller.go | 24 +- config/apinetlet/rbac/role.yaml | 25 ++ 8 files changed, 542 insertions(+), 22 deletions(-) create mode 100644 apinetlet/controllers/helper.go create mode 100644 apinetlet/controllers/natgateway_controller.go create mode 100644 apinetlet/controllers/natgateway_controller_test.go diff --git a/api/v1alpha1/publicip_types.go b/api/v1alpha1/publicip_types.go index bf0e49dc..d8fac1ed 100644 --- a/api/v1alpha1/publicip_types.go +++ b/api/v1alpha1/publicip_types.go @@ -78,6 +78,16 @@ type PublicIP struct { Status PublicIPStatus `json:"status,omitempty"` } +func (ip *PublicIP) IsAllocated() bool { + apiNetPublicIPConditions := ip.Status.Conditions + idx := PublicIPConditionIndex(ip.Status.Conditions, PublicIPAllocated) + if idx < 0 || apiNetPublicIPConditions[idx].Status != corev1.ConditionTrue { + return false + } + + return ip.Spec.IP.IsValid() +} + // +kubebuilder:object:root=true // PublicIPList contains a list of PublicIP. diff --git a/apinetlet/api/v1alpha1/common_types.go b/apinetlet/api/v1alpha1/common_types.go index 39e44335..8f0a9749 100644 --- a/apinetlet/api/v1alpha1/common_types.go +++ b/apinetlet/api/v1alpha1/common_types.go @@ -23,5 +23,9 @@ const ( VirtualIPNamespaceLabel = "apinetlet.api.onmetal.de/virtualip-namespace" VirtualIPNameLabel = "apinetlet.api.onmetal.de/virtualip-name" + NATGatewayUIDLabel = "apinetlet.api.onmetal.de/natgateway-uid" + NATGatewayNamespaceLabel = "apinetlet.api.onmetal.de/natgateway-namespace" + NATGatewayNameLabel = "apinetlet.api.onmetal.de/natgateway-name" + FieldOwner = "apinetlet.api.onmetal.de/field-owner" ) diff --git a/apinetlet/controllers/controllers_suite_test.go b/apinetlet/controllers/controllers_suite_test.go index d1e1f20a..a3d3b681 100644 --- a/apinetlet/controllers/controllers_suite_test.go +++ b/apinetlet/controllers/controllers_suite_test.go @@ -156,6 +156,12 @@ func SetupTest(ctx context.Context) *corev1.Namespace { APINetNamespace: ns.Name, }).SetupWithManager(k8sManager, k8sManager)).To(Succeed()) + Expect((&NatGatewayReconciler{ + Client: k8sManager.GetClient(), + APINetClient: k8sManager.GetClient(), + APINetNamespace: ns.Name, + }).SetupWithManager(k8sManager, k8sManager)).To(Succeed()) + go func() { defer GinkgoRecover() Expect(k8sManager.Start(mgrCtx)).To(Succeed(), "failed to start manager") diff --git a/apinetlet/controllers/helper.go b/apinetlet/controllers/helper.go new file mode 100644 index 00000000..4b8572db --- /dev/null +++ b/apinetlet/controllers/helper.go @@ -0,0 +1,30 @@ +// Copyright 2022 OnMetal authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package controllers + +import ( + onmetalapinetv1alpha1 "github.com/onmetal/onmetal-api-net/api/v1alpha1" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/predicate" +) + +func getApiNetPublicIPAllocationChangedPredicate() predicate.Funcs { + return predicate.Funcs{ + UpdateFunc: func(event event.UpdateEvent) bool { + oldAPINetPublicIP, newAPINetPublicIP := event.ObjectOld.(*onmetalapinetv1alpha1.PublicIP), event.ObjectNew.(*onmetalapinetv1alpha1.PublicIP) + return oldAPINetPublicIP.IsAllocated() != newAPINetPublicIP.IsAllocated() + }, + } +} diff --git a/apinetlet/controllers/natgateway_controller.go b/apinetlet/controllers/natgateway_controller.go new file mode 100644 index 00000000..508c4c51 --- /dev/null +++ b/apinetlet/controllers/natgateway_controller.go @@ -0,0 +1,275 @@ +// Copyright 2022 OnMetal authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package controllers + +import ( + "context" + "fmt" + "net/netip" + "strings" + + "github.com/go-logr/logr" + "github.com/onmetal/controller-utils/clientutils" + onmetalapinetv1alpha1 "github.com/onmetal/onmetal-api-net/api/v1alpha1" + apinetletv1alpha1 "github.com/onmetal/onmetal-api-net/apinetlet/api/v1alpha1" + commonv1alpha1 "github.com/onmetal/onmetal-api/api/common/v1alpha1" + networkingv1alpha1 "github.com/onmetal/onmetal-api/api/networking/v1alpha1" + "github.com/onmetal/onmetal-api/apiutils/predicates" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/cluster" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/source" +) + +const ( + natGatewayFinalizer = "apinet.api.onmetal.de/natgateway" +) + +type NatGatewayReconciler struct { + client.Client + APINetClient client.Client + + APINetNamespace string + + WatchFilterValue string +} + +//+kubebuilder:rbac:groups="",resources=events,verbs=create;patch +//+kubebuilder:rbac:groups=networking.api.onmetal.de,resources=natgateways,verbs=get;list;watch;update;patch +//+kubebuilder:rbac:groups=networking.api.onmetal.de,resources=natgateways/finalizers,verbs=update;patch +//+kubebuilder:rbac:groups=networking.api.onmetal.de,resources=natgateways/status,verbs=get;update;patch + +func (r *NatGatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + log := ctrl.LoggerFrom(ctx) + natGateway := &networkingv1alpha1.NATGateway{} + if err := r.Get(ctx, req.NamespacedName, natGateway); err != nil { + if !apierrors.IsNotFound(err) { + return ctrl.Result{}, fmt.Errorf("error getting nat gateway %s: %w", req.NamespacedName, err) + } + + return r.deleteGone(ctx, log, req.NamespacedName) + } + + return r.reconcileExists(ctx, log, natGateway) +} + +func (r *NatGatewayReconciler) deleteGone(ctx context.Context, log logr.Logger, natGatewayKey client.ObjectKey) (ctrl.Result, error) { + log.V(1).Info("Delete gone") + + log.V(1).Info("Deleting any matching apinet public ips") + if err := r.APINetClient.DeleteAllOf(ctx, &onmetalapinetv1alpha1.PublicIP{}, + client.InNamespace(r.APINetNamespace), + client.MatchingLabels{ + apinetletv1alpha1.NATGatewayNamespaceLabel: natGatewayKey.Namespace, + apinetletv1alpha1.NATGatewayNameLabel: natGatewayKey.Name, + }, + ); err != nil { + return ctrl.Result{}, fmt.Errorf("error deleting apinet public ips: %w", err) + } + + log.V(1).Info("Issued delete for any leftover apinet public ip") + return ctrl.Result{}, nil +} + +func (r *NatGatewayReconciler) reconcileExists(ctx context.Context, log logr.Logger, natGateway *networkingv1alpha1.NATGateway) (ctrl.Result, error) { + log = log.WithValues("UID", natGateway.UID) + if !natGateway.DeletionTimestamp.IsZero() { + return r.delete(ctx, log, natGateway) + } + return r.reconcile(ctx, log, natGateway) +} + +func (r *NatGatewayReconciler) delete(ctx context.Context, log logr.Logger, natGateway *networkingv1alpha1.NATGateway) (ctrl.Result, error) { + log.V(1).Info("Delete") + + if !controllerutil.ContainsFinalizer(natGateway, natGatewayFinalizer) { + log.V(1).Info("No finalizer present, nothing to do") + return ctrl.Result{}, nil + } + + var count int + for _, ipFamily := range natGateway.Spec.IPFamilies { + for _, ip := range natGateway.Spec.IPs { + if err := r.APINetClient.Delete(ctx, &onmetalapinetv1alpha1.PublicIP{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: r.APINetNamespace, + Name: fmt.Sprintf("%s-%s-%s", natGateway.UID, ip.Name, strings.ToLower(string(ipFamily))), + }, + }); err != nil { + if !apierrors.IsNotFound(err) { + return ctrl.Result{}, fmt.Errorf("error deleting target public ip: %w", err) + } + count++ + + } + + } + } + + if count < len(natGateway.Spec.IPs)*len(natGateway.Spec.IPFamilies) { + log.V(1).Info("Target public ip is not yet gone, requeueing") + return ctrl.Result{Requeue: true}, nil + } + + log.V(1).Info("Target public ip is gone, removing finalizer") + if err := clientutils.PatchRemoveFinalizer(ctx, r.Client, natGateway, natGatewayFinalizer); err != nil { + return ctrl.Result{}, fmt.Errorf("error removing finalizer: %w", err) + } + log.V(1).Info("Removed finalizer") + return ctrl.Result{}, nil +} + +func (r *NatGatewayReconciler) reconcile(ctx context.Context, log logr.Logger, natGateway *networkingv1alpha1.NATGateway) (ctrl.Result, error) { + log.V(1).Info("Reconcile") + + log.V(1).Info("Ensuring finalizer") + modified, err := clientutils.PatchEnsureFinalizer(ctx, r.Client, natGateway, natGatewayFinalizer) + if err != nil { + return ctrl.Result{}, fmt.Errorf("error ensuring finalizer: %w", err) + } + if modified { + log.V(1).Info("Added finalizer, requeueing") + return ctrl.Result{Requeue: true}, nil + } + + ips, err := r.applyPublicIPs(ctx, log, natGateway) + if err != nil { + return ctrl.Result{}, fmt.Errorf("error getting / applying public ip: %w", err) + } + + if err := r.patchStatus(ctx, log, natGateway, ips); err != nil { + return ctrl.Result{}, fmt.Errorf("error patching nat gateway status") + } + + log.V(1).Info("Patched nat gateway status") + return ctrl.Result{}, nil +} + +func (r *NatGatewayReconciler) applyPublicIP(ctx context.Context, log logr.Logger, natGateway *networkingv1alpha1.NATGateway, ipName string, ipFamily corev1.IPFamily) (netip.Addr, error) { + apiNetPublicIP := &onmetalapinetv1alpha1.PublicIP{ + TypeMeta: metav1.TypeMeta{ + APIVersion: onmetalapinetv1alpha1.GroupVersion.String(), + Kind: "PublicIP", + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: r.APINetNamespace, + Name: fmt.Sprintf("%s-%s-%s", natGateway.UID, ipName, strings.ToLower(string(ipFamily))), + Labels: map[string]string{ + apinetletv1alpha1.NATGatewayNamespaceLabel: natGateway.Namespace, + apinetletv1alpha1.NATGatewayNameLabel: natGateway.Name, + apinetletv1alpha1.NATGatewayUIDLabel: string(natGateway.UID), + }, + }, + Spec: onmetalapinetv1alpha1.PublicIPSpec{ + IPFamily: ipFamily, + }, + } + + log.V(1).Info("Applying apinet public ip", "ipName", ipName, "ipFamily", ipFamily) + if err := r.APINetClient.Patch(ctx, apiNetPublicIP, client.Apply, + client.FieldOwner(apinetletv1alpha1.FieldOwner), + client.ForceOwnership, + ); err != nil { + return netip.Addr{}, fmt.Errorf("error applying apinet public ip: %w", err) + } + log.V(1).Info("Applied apinet public ip") + + if !apiNetPublicIP.IsAllocated() { + return netip.Addr{}, nil + } + ip := apiNetPublicIP.Spec.IP + return ip.Addr, nil +} + +func (r *NatGatewayReconciler) applyPublicIPs(ctx context.Context, log logr.Logger, natGateway *networkingv1alpha1.NATGateway) (map[string]netip.Addr, error) { + ips := map[string]netip.Addr{} + for _, ipFamily := range natGateway.Spec.IPFamilies { + for _, ip := range natGateway.Spec.IPs { + apiNetPublicIP, err := r.applyPublicIP(ctx, log, natGateway, ip.Name, ipFamily) + if err != nil { + return nil, err + } + ips[ip.Name] = apiNetPublicIP + } + } + return ips, nil +} + +func (r *NatGatewayReconciler) patchStatus(ctx context.Context, log logr.Logger, natGateway *networkingv1alpha1.NATGateway, ips map[string]netip.Addr) error { + base := natGateway.DeepCopy() + natGateway.Status.IPs = []networkingv1alpha1.NATGatewayIPStatus{} + + for ipName, ip := range ips { + if !ip.IsValid() { + log.V(2).Info("Public ip is not yet allocated", "ipName", ipName) + continue + } + + log.V(2).Info("Public ip is allocated", "ipName", ipName) + natGateway.Status.IPs = append(natGateway.Status.IPs, networkingv1alpha1.NATGatewayIPStatus{ + Name: ipName, + IP: commonv1alpha1.IP{ + Addr: ip, + }, + }) + } + + return r.Status().Patch(ctx, natGateway, client.MergeFrom(base)) +} + +func (r *NatGatewayReconciler) SetupWithManager(mgr ctrl.Manager, apiNetCluster cluster.Cluster) error { + log := ctrl.Log.WithName("natgateway").WithName("setup") + + return ctrl.NewControllerManagedBy(mgr). + For( + &networkingv1alpha1.NATGateway{}, + builder.WithPredicates( + predicates.ResourceHasFilterLabel(log, r.WatchFilterValue), + predicates.ResourceIsNotExternallyManaged(log), + ), + ). + Watches( + source.NewKindWithCache(&onmetalapinetv1alpha1.PublicIP{}, apiNetCluster.GetCache()), + handler.EnqueueRequestsFromMapFunc(func(obj client.Object) []ctrl.Request { + apiNetPublicIP := obj.(*onmetalapinetv1alpha1.PublicIP) + + if apiNetPublicIP.Namespace != r.APINetNamespace { + return nil + } + + namespace, ok := apiNetPublicIP.Labels[apinetletv1alpha1.NATGatewayNamespaceLabel] + if !ok { + return nil + } + + name, ok := apiNetPublicIP.Labels[apinetletv1alpha1.NATGatewayNameLabel] + if !ok { + return nil + } + + return []ctrl.Request{{NamespacedName: client.ObjectKey{Namespace: namespace, Name: name}}} + }), + builder.WithPredicates( + getApiNetPublicIPAllocationChangedPredicate(), + ), + ). + Complete(r) +} diff --git a/apinetlet/controllers/natgateway_controller_test.go b/apinetlet/controllers/natgateway_controller_test.go new file mode 100644 index 00000000..8b0db8b7 --- /dev/null +++ b/apinetlet/controllers/natgateway_controller_test.go @@ -0,0 +1,190 @@ +// Copyright 2022 OnMetal authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package controllers + +import ( + "fmt" + "strings" + + onmetalapinetv1alpha1 "github.com/onmetal/onmetal-api-net/api/v1alpha1" + apinetletv1alpha1 "github.com/onmetal/onmetal-api-net/apinetlet/api/v1alpha1" + commonv1alpha1 "github.com/onmetal/onmetal-api/api/common/v1alpha1" + networkingv1alpha1 "github.com/onmetal/onmetal-api/api/networking/v1alpha1" + "github.com/onmetal/onmetal-api/testutils" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + . "sigs.k8s.io/controller-runtime/pkg/envtest/komega" +) + +var _ = Describe("NATGatewayController", func() { + ctx := testutils.SetupContext() + ns := SetupTest(ctx) + + It("should allocate a public ip", func() { + By("creating a network") + network := &networkingv1alpha1.Network{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns.Name, + GenerateName: "network-", + }, + } + Expect(k8sClient.Create(ctx, network)).To(Succeed()) + + ipName := "ip" + ipFamily := corev1.IPv4Protocol + + By("creating a nat gateway") + natGateway := &networkingv1alpha1.NATGateway{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns.Name, + GenerateName: "nat-gateway-ip-", + }, + Spec: networkingv1alpha1.NATGatewaySpec{ + Type: networkingv1alpha1.NATGatewayTypePublic, + IPFamilies: []corev1.IPFamily{ + ipFamily, + }, + NetworkRef: corev1.LocalObjectReference{Name: network.Name}, + IPs: []networkingv1alpha1.NATGatewayIP{ + { + Name: ipName, + }, + }, + }, + } + Expect(k8sClient.Create(ctx, natGateway)).To(Succeed()) + + By("waiting for the corresponding public ip to be created") + publicIP := &onmetalapinetv1alpha1.PublicIP{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns.Name, + Name: fmt.Sprintf("%s-%s-%s", natGateway.UID, ipName, strings.ToLower(string(ipFamily))), + }, + } + Eventually(Get(publicIP)).Should(Succeed()) + + By("inspecting the created public ip") + Expect(publicIP.Labels).To(Equal(map[string]string{ + apinetletv1alpha1.NATGatewayNamespaceLabel: natGateway.Namespace, + apinetletv1alpha1.NATGatewayNameLabel: natGateway.Name, + apinetletv1alpha1.NATGatewayUIDLabel: string(natGateway.UID), + })) + Expect(publicIP.Spec).To(Equal(onmetalapinetv1alpha1.PublicIPSpec{ + IPFamily: ipFamily, + })) + + By("asserting the nat gateway does not get an ip address") + Consistently(Object(natGateway)).Should(HaveField("Status.IPs", BeNil())) + + By("patching the public ip spec ips") + basePublicIP := publicIP.DeepCopy() + publicIP.Spec.IP = onmetalapinetv1alpha1.MustParseNewIP("10.0.0.1") + Expect(k8sClient.Patch(ctx, publicIP, client.MergeFrom(basePublicIP))).To(Succeed()) + + By("patching the public ip status to allocated") + basePublicIP = publicIP.DeepCopy() + onmetalapinetv1alpha1.SetPublicIPCondition(&publicIP.Status.Conditions, onmetalapinetv1alpha1.PublicIPCondition{ + Type: onmetalapinetv1alpha1.PublicIPAllocated, + Status: corev1.ConditionTrue, + }) + Expect(k8sClient.Status().Patch(ctx, publicIP, client.MergeFrom(basePublicIP))).To(Succeed()) + + By("checking that nat gateway contains ip") + Eventually(Object(natGateway)).Should(HaveField("Status.IPs", + ContainElement(networkingv1alpha1.NATGatewayIPStatus{ + Name: ipName, + IP: *commonv1alpha1.MustParseNewIP("10.0.0.1"), + }), + )) + + By("requesting one more ip") + natGatewayBase := natGateway.DeepCopy() + natGateway.Spec.IPs = append(natGateway.Spec.IPs, networkingv1alpha1.NATGatewayIP{ + Name: ipName + "-2", + }) + Expect(k8sClient.Patch(ctx, natGateway, client.MergeFrom(natGatewayBase))).To(Succeed()) + + By("waiting for the corresponding public ip to be created") + publicIP2 := &onmetalapinetv1alpha1.PublicIP{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns.Name, + Name: fmt.Sprintf("%s-%s-%s", natGateway.UID, ipName+"-2", strings.ToLower(string(ipFamily))), + }, + } + Eventually(Get(publicIP2)).Should(Succeed()) + + By("patching the second public ip spec ips") + basePublicIP2 := publicIP2.DeepCopy() + publicIP2.Spec.IP = onmetalapinetv1alpha1.MustParseNewIP("10.0.0.2") + Expect(k8sClient.Patch(ctx, publicIP2, client.MergeFrom(basePublicIP2))).To(Succeed()) + + By("patching the second public ip status to allocated") + basePublicIP2 = publicIP2.DeepCopy() + onmetalapinetv1alpha1.SetPublicIPCondition(&publicIP2.Status.Conditions, onmetalapinetv1alpha1.PublicIPCondition{ + Type: onmetalapinetv1alpha1.PublicIPAllocated, + Status: corev1.ConditionTrue, + }) + Expect(k8sClient.Status().Patch(ctx, publicIP2, client.MergeFrom(basePublicIP2))).To(Succeed()) + + By("checking that nat gateway contains ip") + Eventually(Object(natGateway)).Should(HaveField("Status.IPs", + ContainElements( + networkingv1alpha1.NATGatewayIPStatus{ + Name: ipName, + IP: *commonv1alpha1.MustParseNewIP("10.0.0.1"), + }, + networkingv1alpha1.NATGatewayIPStatus{ + Name: ipName + "-2", + IP: *commonv1alpha1.MustParseNewIP("10.0.0.2"), + }), + )) + + By("deleting the nat gateway") + Expect(k8sClient.Delete(ctx, natGateway)).To(Succeed()) + + By("waiting for it to be gone") + Eventually(Get(natGateway)).Should(Satisfy(apierrors.IsNotFound)) + + By("asserting the corresponding public ips are gone as well") + Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(publicIP), publicIP)).To(Satisfy(apierrors.IsNotFound)) + Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(publicIP2), publicIP2)).To(Satisfy(apierrors.IsNotFound)) + }) + + It("should clean up dangling public ips", func() { + By("creating a public ip") + publicIP := &onmetalapinetv1alpha1.PublicIP{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns.Name, + GenerateName: "public-ip-", + Labels: map[string]string{ + apinetletv1alpha1.NATGatewayNamespaceLabel: ns.Name, + apinetletv1alpha1.NATGatewayNameLabel: "some-name", + apinetletv1alpha1.NATGatewayUIDLabel: "some-uid", + }, + }, + Spec: onmetalapinetv1alpha1.PublicIPSpec{ + IPFamily: corev1.IPv4Protocol, + }, + } + Expect(k8sClient.Create(ctx, publicIP)).To(Succeed()) + + By("waiting for the public ip to be gone") + Eventually(Get(publicIP)).Should(Satisfy(apierrors.IsNotFound)) + }) +}) diff --git a/apinetlet/controllers/virtualip_controller.go b/apinetlet/controllers/virtualip_controller.go index 8f10d3c5..55694bdd 100644 --- a/apinetlet/controllers/virtualip_controller.go +++ b/apinetlet/controllers/virtualip_controller.go @@ -26,7 +26,6 @@ import ( commonv1alpha1 "github.com/onmetal/onmetal-api/api/common/v1alpha1" networkingv1alpha1 "github.com/onmetal/onmetal-api/api/networking/v1alpha1" "github.com/onmetal/onmetal-api/apiutils/predicates" - corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" @@ -34,9 +33,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/cluster" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/handler" - "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/source" ) @@ -170,16 +167,6 @@ func (r *VirtualIPReconciler) reconcile(ctx context.Context, log logr.Logger, vi return ctrl.Result{}, nil } -func isAPINetPublicIPAllocated(apiNetPublicIP *onmetalapinetv1alpha1.PublicIP) bool { - apiNetPublicIPConditions := apiNetPublicIP.Status.Conditions - idx := onmetalapinetv1alpha1.PublicIPConditionIndex(apiNetPublicIP.Status.Conditions, onmetalapinetv1alpha1.PublicIPAllocated) - if idx < 0 || apiNetPublicIPConditions[idx].Status != corev1.ConditionTrue { - return false - } - - return apiNetPublicIP.Spec.IP.IsValid() -} - func (r *VirtualIPReconciler) applyPublicIP(ctx context.Context, log logr.Logger, virtualIP *networkingv1alpha1.VirtualIP) (netip.Addr, error) { apiNetPublicIP := &onmetalapinetv1alpha1.PublicIP{ TypeMeta: metav1.TypeMeta{ @@ -209,7 +196,7 @@ func (r *VirtualIPReconciler) applyPublicIP(ctx context.Context, log logr.Logger } log.V(1).Info("Applied apinet public ip") - if !isAPINetPublicIPAllocated(apiNetPublicIP) { + if !apiNetPublicIP.IsAllocated() { return netip.Addr{}, nil } ip := apiNetPublicIP.Spec.IP @@ -234,13 +221,6 @@ func (r *VirtualIPReconciler) patchStatusUnallocated(ctx context.Context, virtua return nil } -var apiNetPublicIPAllocationChanged = predicate.Funcs{ - UpdateFunc: func(event event.UpdateEvent) bool { - oldAPINetPublicIP, newAPINetPublicIP := event.ObjectOld.(*onmetalapinetv1alpha1.PublicIP), event.ObjectNew.(*onmetalapinetv1alpha1.PublicIP) - return isAPINetPublicIPAllocated(oldAPINetPublicIP) != isAPINetPublicIPAllocated(newAPINetPublicIP) - }, -} - func (r *VirtualIPReconciler) SetupWithManager(mgr ctrl.Manager, apiNetCluster cluster.Cluster) error { log := ctrl.Log.WithName("virtualip").WithName("setup") @@ -274,7 +254,7 @@ func (r *VirtualIPReconciler) SetupWithManager(mgr ctrl.Manager, apiNetCluster c return []ctrl.Request{{NamespacedName: client.ObjectKey{Namespace: namespace, Name: name}}} }), builder.WithPredicates( - apiNetPublicIPAllocationChanged, + getApiNetPublicIPAllocationChangedPredicate(), ), ). Complete(r) diff --git a/config/apinetlet/rbac/role.yaml b/config/apinetlet/rbac/role.yaml index 5f335836..4d2314f8 100644 --- a/config/apinetlet/rbac/role.yaml +++ b/config/apinetlet/rbac/role.yaml @@ -12,6 +12,31 @@ rules: verbs: - create - patch +- apiGroups: + - networking.api.onmetal.de + resources: + - natgateways + verbs: + - get + - list + - patch + - update + - watch +- apiGroups: + - networking.api.onmetal.de + resources: + - natgateways/finalizers + verbs: + - patch + - update +- apiGroups: + - networking.api.onmetal.de + resources: + - natgateways/status + verbs: + - get + - patch + - update - apiGroups: - networking.api.onmetal.de resources: From 201ea8a5c3473ffff72f5429324d6001e685979e Mon Sep 17 00:00:00 2001 From: Lukas Frank Date: Tue, 6 Dec 2022 15:30:40 +0100 Subject: [PATCH 2/2] Fix `RBAC` --- apinetlet/controllers/natgateway_controller.go | 2 ++ apinetlet/controllers/virtualip_controller.go | 2 ++ config/apinetlet/rbac/role.yaml | 18 ++++++++++++++++++ 3 files changed, 22 insertions(+) diff --git a/apinetlet/controllers/natgateway_controller.go b/apinetlet/controllers/natgateway_controller.go index 508c4c51..974afcc5 100644 --- a/apinetlet/controllers/natgateway_controller.go +++ b/apinetlet/controllers/natgateway_controller.go @@ -56,6 +56,8 @@ type NatGatewayReconciler struct { //+kubebuilder:rbac:groups=networking.api.onmetal.de,resources=natgateways,verbs=get;list;watch;update;patch //+kubebuilder:rbac:groups=networking.api.onmetal.de,resources=natgateways/finalizers,verbs=update;patch //+kubebuilder:rbac:groups=networking.api.onmetal.de,resources=natgateways/status,verbs=get;update;patch +//+kubebuilder:rbac:groups=apinet.api.onmetal.de,resources=publicips,verbs=get;list;watch;create;update;patch;delete +//+kubebuilder:rbac:groups=apinet.api.onmetal.de,resources=publicips/status,verbs=get func (r *NatGatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { log := ctrl.LoggerFrom(ctx) diff --git a/apinetlet/controllers/virtualip_controller.go b/apinetlet/controllers/virtualip_controller.go index 55694bdd..ecf82f6c 100644 --- a/apinetlet/controllers/virtualip_controller.go +++ b/apinetlet/controllers/virtualip_controller.go @@ -54,6 +54,8 @@ type VirtualIPReconciler struct { //+kubebuilder:rbac:groups=networking.api.onmetal.de,resources=virtualips,verbs=get;list;watch;update;patch //+kubebuilder:rbac:groups=networking.api.onmetal.de,resources=virtualips/finalizers,verbs=update;patch //+kubebuilder:rbac:groups=networking.api.onmetal.de,resources=virtualips/status,verbs=get;update;patch +//+kubebuilder:rbac:groups=apinet.api.onmetal.de,resources=publicips,verbs=get;list;watch;create;update;patch;delete +//+kubebuilder:rbac:groups=apinet.api.onmetal.de,resources=publicips/status,verbs=get func (r *VirtualIPReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { log := ctrl.LoggerFrom(ctx) diff --git a/config/apinetlet/rbac/role.yaml b/config/apinetlet/rbac/role.yaml index 4d2314f8..654fbb6c 100644 --- a/config/apinetlet/rbac/role.yaml +++ b/config/apinetlet/rbac/role.yaml @@ -12,6 +12,24 @@ rules: verbs: - create - patch +- apiGroups: + - apinet.api.onmetal.de + resources: + - publicips + verbs: + - create + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - apinet.api.onmetal.de + resources: + - publicips/status + verbs: + - get - apiGroups: - networking.api.onmetal.de resources: