add tests, docs and fix lint

README.md

@@ -194,6 +194,13 @@ metadata:
     # If this is set to a different network ID than defined as default in the yawol-cloud-controller
     # the default from the yawol-cloud-controller will be added to the additionalNetworks.
     yawol.stackit.cloud/defaultNetworkID: "OS-networkID"
+    # If set to true it do not add the default network ID from
+    # the yawol-cloud-controller to the additionalNetworks.
+    yawol.stackit.cloud/skipCloudControllerDefaultNetworkID: "false"
+    # Overwrites the projectID which is set by the secret.
+    # If not set the settings from the secret binding will be used.
+    # This field is immutable and can not be changed after the service is created.
+    yawol.stackit.cloud/projectID: "OS-ProjectID"
     # Overwrites the openstack floating network for the loadbalancer.
     yawol.stackit.cloud/floatingNetworkID: "OS-floatingNetID"
     # Override the default OpenStack availability zone.

controllers/yawol-cloud-controller/targetcontroller/service_controller.go

@@ -240,7 +240,7 @@ func (r *ServiceReconciler) reconcileInfrastructure(
 	}
 
 	if !reflect.DeepEqual(newInfra.ProjectID, lb.Spec.Infrastructure.ProjectID) {
-		return helper.ErrProjectIsImmutable
+		return kubernetes.SendErrorAsEvent(r.Recorder, helper.ErrProjectIsImmutable, svc)
 	}
 
 	if !reflect.DeepEqual(newInfra, lb.Spec.Infrastructure) {

controllers/yawol-cloud-controller/targetcontroller/service_controller_test.go

@@ -2108,5 +2108,159 @@ var _ = Describe("Check loadbalancer reconcile", func() {
 				return fmt.Errorf("additionalNetwork are still set %v", lb.Spec.Infrastructure.AdditionalNetworks)
 			}, time.Second*5, time.Millisecond*500).Should(Succeed())
 		})
+
+		It("should update the skipCloudControllerDefaultNetworkID field", func() {
+			By("creating a service without skipCloudControllerDefaultNetworkID")
+			service := v1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "service-test31",
+					Namespace: "default",
+					Annotations: map[string]string{
+						yawolv1beta1.ServiceDefaultNetworkID: "default-networkID",
+					},
+				},
+				Spec: v1.ServiceSpec{
+					Ports: []v1.ServicePort{
+						{
+							Name:       "port1",
+							Protocol:   v1.ProtocolTCP,
+							Port:       12345,
+							TargetPort: intstr.IntOrString{IntVal: 12345},
+							NodePort:   31031,
+						},
+					},
+					Type: "LoadBalancer",
+				}}
+			Expect(k8sClient.Create(ctx, &service)).Should(Succeed())
+
+			By("checking that defaultNetworkID is in additional networks")
+			Eventually(func() error {
+				err := k8sClient.Get(ctx, types.NamespacedName{Name: "default--service-test31", Namespace: "default"}, &lb)
+				if err != nil {
+					return err
+				}
+				if len(lb.Spec.Infrastructure.AdditionalNetworks) == 1 &&
+					lb.Spec.Infrastructure.AdditionalNetworks[0].NetworkID == *testInfraDefaults.NetworkID {
+					return nil
+				}
+				return fmt.Errorf("additionalNetwork is not set with defaultNetwork %v", lb.Spec.Infrastructure.AdditionalNetworks)
+			}, time.Second*5, time.Millisecond*500).Should(Succeed())
+
+			By("update svc to add skipCloudControllerDefaultNetworkID")
+			Expect(k8sClient.Get(ctx, client.ObjectKey{Name: service.Name, Namespace: service.Namespace}, &service)).Should(Succeed())
+			service.ObjectMeta.Annotations = map[string]string{
+				yawolv1beta1.ServiceDefaultNetworkID:                    "default-networkID",
+				yawolv1beta1.ServiceSkipCloudControllerDefaultNetworkID: "true",
+			}
+			Expect(k8sClient.Update(ctx, &service)).Should(Succeed())
+
+			By("check if default network is not present in additionalNetworks")
+			Eventually(func() error {
+				err := k8sClient.Get(ctx, types.NamespacedName{Name: "default--service-test31", Namespace: "default"}, &lb)
+				if err != nil {
+					return err
+				}
+				if len(lb.Spec.Infrastructure.AdditionalNetworks) == 0 {
+					return nil
+				}
+				return fmt.Errorf("additionalNetworks are not correct %v", lb.Spec.Infrastructure.AdditionalNetworks)
+			}, time.Second*5, time.Millisecond*500).Should(Succeed())
+
+			By("update svc to disable skipCloudControllerDefaultNetworkID")
+			Expect(k8sClient.Get(ctx, client.ObjectKey{Name: service.Name, Namespace: service.Namespace}, &service)).Should(Succeed())
+			service.ObjectMeta.Annotations = map[string]string{
+				yawolv1beta1.ServiceDefaultNetworkID: "default-networkID",
+			}
+			Expect(k8sClient.Update(ctx, &service)).Should(Succeed())
+
+			By("checking that defaultNetworkID is in additional networks")
+			Eventually(func() error {
+				err := k8sClient.Get(ctx, types.NamespacedName{Name: "default--service-test31", Namespace: "default"}, &lb)
+				if err != nil {
+					return err
+				}
+				if len(lb.Spec.Infrastructure.AdditionalNetworks) == 1 &&
+					lb.Spec.Infrastructure.AdditionalNetworks[0].NetworkID == *testInfraDefaults.NetworkID {
+					return nil
+				}
+				return fmt.Errorf("additionalNetwork is not set with defaultNetwork %v", lb.Spec.Infrastructure.AdditionalNetworks)
+			}, time.Second*5, time.Millisecond*500).Should(Succeed())
+		})
+
+		It("should be possible to add a project - but it is immutable", func() {
+			By("creating a service with different project")
+			service := v1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "service-test32",
+					Namespace: "default",
+					Annotations: map[string]string{
+						yawolv1beta1.ServiceDefaultProjectID: "otherProject",
+					},
+				},
+				Spec: v1.ServiceSpec{
+					Ports: []v1.ServicePort{
+						{
+							Name:       "port1",
+							Protocol:   v1.ProtocolTCP,
+							Port:       12345,
+							TargetPort: intstr.IntOrString{IntVal: 12345},
+							NodePort:   31032,
+						},
+					},
+					Type: "LoadBalancer",
+				}}
+			Expect(k8sClient.Create(ctx, &service)).Should(Succeed())
+
+			By("checking that projectID is set")
+			Eventually(func() error {
+				err := k8sClient.Get(ctx, types.NamespacedName{Name: "default--service-test32", Namespace: "default"}, &lb)
+				if err != nil {
+					return err
+				}
+				if lb.Spec.Infrastructure.ProjectID != nil &&
+					*lb.Spec.Infrastructure.ProjectID == "otherProject" {
+					return nil
+				}
+				return fmt.Errorf("projectID is not set in infrastructure %v", lb.Spec.Infrastructure.ProjectID)
+			}, time.Second*5, time.Millisecond*500).Should(Succeed())
+
+			By("update svc with other project - should not work")
+			Expect(k8sClient.Get(ctx, client.ObjectKey{Name: service.Name, Namespace: service.Namespace}, &service)).Should(Succeed())
+			service.ObjectMeta.Annotations = map[string]string{
+				yawolv1beta1.ServiceDefaultProjectID: "otherProject2",
+			}
+			Expect(k8sClient.Update(ctx, &service)).Should(Succeed())
+
+			By("check for error event")
+			Eventually(func() error {
+				eventList := v1.EventList{}
+				err := k8sClient.List(ctx, &eventList)
+				if err != nil {
+					return err
+				}
+				for _, event := range eventList.Items {
+					if event.InvolvedObject.Name == "service-test32" &&
+						event.InvolvedObject.Kind == "Service" &&
+						event.Type == v1.EventTypeWarning &&
+						strings.Contains(event.Message, helper.ErrProjectIsImmutable.Error()) {
+						return nil
+					}
+				}
+				return helper.ErrNoEventFound
+			}, time.Second*5, time.Millisecond*500).Should(Succeed())
+
+			By("checking that projectID is still the ID from the creation")
+			Eventually(func() error {
+				err := k8sClient.Get(ctx, types.NamespacedName{Name: "default--service-test32", Namespace: "default"}, &lb)
+				if err != nil {
+					return err
+				}
+				if lb.Spec.Infrastructure.ProjectID != nil &&
+					*lb.Spec.Infrastructure.ProjectID == "otherProject" {
+					return nil
+				}
+				return fmt.Errorf("projectID is not correctly set in infrastructure %v", lb.Spec.Infrastructure.ProjectID)
+			}, time.Second*5, time.Millisecond*500).Should(Succeed())
+		})
 	})
 })

controllers/yawol-controller/loadbalancer/loadbalancer_controller.go

@@ -374,7 +374,7 @@ func (r *Reconciler) assignOrCreateFIP(
 	return nil
 }
 
-func (r *Reconciler) reconcilePort(
+func (r *Reconciler) reconcilePort( //nolint: gocyclo // TODO reduce complexity in future
 	ctx context.Context,
 	req ctrl.Request,
 	lb *yawolv1beta1.LoadBalancer,
@@ -1106,7 +1106,6 @@ func (r *Reconciler) deletePorts(
 
 	var requeue bool
 
-	//nolint: dupl // we can't extract this code because of generics
 	if lb.Status.PortID != nil {
 		port, err := openstackhelper.GetPortByID(ctx, portClient, *lb.Status.PortID)
 		if err != nil {
@@ -1132,6 +1131,7 @@ func (r *Reconciler) deletePorts(
 	}
 
 	// clean up orphan ports
+	//nolint: dupl // we can't extract this code because of generics
 	if lb.Status.PortName != nil {
 		portName := *lb.Status.PortName
 		var portList []ports.Port
@@ -1221,6 +1221,7 @@ func (r *Reconciler) deleteSecGroups(
 	}
 
 	// clean up orphan secgroups
+	//nolint: dupl // we can't extract this code because of generics
 	if lb.Status.SecurityGroupName != nil {
 		secGroupName := *lb.Status.SecurityGroupName
 		var secGroupList []groups.SecGroup

controllers/yawol-controller/loadbalancer/loadbalancer_controller_test.go

@@ -103,12 +103,14 @@ var _ = Describe("loadbalancer controller", func() {
 				g.Expect(act.Status.PortID).ToNot(BeNil())
 				g.Expect(act.Status.PortName).ToNot(BeNil())
 
+				g.Expect(act.Status.PortIP).ToNot(BeNil())
+				g.Expect(*act.Status.PortIP).ToNot(Equal(""))
+
 				g.Expect(act.Status.FloatingID).To(BeNil())
 				g.Expect(act.Status.FloatingName).To(BeNil())
 
 				g.Expect(act.Status.ExternalIP).ToNot(BeNil())
 				g.Expect(*act.Status.ExternalIP).ToNot(Equal(""))
-
 				return nil
 			})
 		})
@@ -118,11 +120,17 @@ var _ = Describe("loadbalancer controller", func() {
 		It("should swap to an internal lb", func() {
 			By("checking that the lb gets created with an public ip")
 			hopefully(lbNN, func(g Gomega, act LB) error {
+				g.Expect(act.Status.PortID).ToNot(BeNil())
+				g.Expect(act.Status.PortName).ToNot(BeNil())
+
+				g.Expect(act.Status.PortIP).ToNot(BeNil())
+				g.Expect(*act.Status.PortIP).ToNot(Equal(""))
+
 				g.Expect(act.Status.FloatingID).ToNot(BeNil())
 				g.Expect(act.Status.FloatingName).ToNot(BeNil())
 
 				g.Expect(act.Status.ExternalIP).ToNot(BeNil())
-
+				g.Expect(*act.Status.ExternalIP).ToNot(Equal(""))
 				return nil
 			})
 

controllers/yawol-controller/loadbalancermachine/loadbalancermachine_controller.go

@@ -341,7 +341,7 @@ func (r *LoadBalancerMachineReconciler) reconcileRoleBinding(
 	return nil
 }
 
-func (r *LoadBalancerMachineReconciler) reconcilePort(
+func (r *LoadBalancerMachineReconciler) reconcilePort( //nolint: gocyclo // TODO reduce complexity in future
 	ctx context.Context,
 	osClient os.Client,
 	req ctrl.Request,

internal/helper/errors.go

@@ -65,5 +65,5 @@ var (
 	ErrCouldNotParseSourceRange              = errors.New("could not parse LoadBalancerSourceRange")
 	ErrListingChildLBMs                      = errors.New("unable to list child loadbalancerMachines")
 	ErrUnsupportedProtocol                   = errors.New("unsupported protocol used (TCP and UDP is supported)")
-	ErrProjectIsImmutable                    = errors.New("project id is immutable, cant be changed after inital creation")
+	ErrProjectIsImmutable                    = errors.New("project id is immutable, cant be changed after initial creation")
 )

internal/helper/openstack/client.go

@@ -3,6 +3,7 @@ package openstack
 import (
 	"context"
 	"fmt"
+
 	yawolv1beta1 "github.com/stackitcloud/yawol/api/v1beta1"
 
 	"github.com/stackitcloud/yawol/internal/openstack"

internal/openstack/client.go

@@ -43,7 +43,12 @@ type OSClient struct {
 //	username="itmyuser"
 //	password="suupersecret"
 //	region="eu01"
-func (r *OSClient) Configure(iniBytes []byte, overwrite OSClientOverwrite, timeout time.Duration, promCounter *prometheus.CounterVec) error {
+func (r *OSClient) Configure(
+	iniBytes []byte,
+	overwrite OSClientOverwrite,
+	timeout time.Duration,
+	promCounter *prometheus.CounterVec,
+) error {
 	r.ini = iniBytes
 	r.timeout = timeout
 	r.promCounter = promCounter
@@ -163,7 +168,12 @@ func (r *OSClient) KeyPairClient(ctx context.Context) (KeyPairClient, error) {
 	return client.Configure(r.computeV2, r.timeout, r.promCounter), nil
 }
 
-func createNetworkV2FromIni(ctx context.Context, iniData []byte, overwrite OSClientOverwrite, timeout time.Duration) (*gophercloud.ServiceClient, error) {
+func createNetworkV2FromIni(
+	ctx context.Context,
+	iniData []byte,
+	overwrite OSClientOverwrite,
+	timeout time.Duration,
+) (*gophercloud.ServiceClient, error) {
 	provider, opts, err := getProvider(ctx, iniData, overwrite, timeout)
 	if err != nil {
 		return nil, err
@@ -177,7 +187,12 @@ func createNetworkV2FromIni(ctx context.Context, iniData []byte, overwrite OSCli
 	return client, nil
 }
 
-func createComputeV2FromIni(ctx context.Context, iniData []byte, overwrite OSClientOverwrite, timeout time.Duration) (*gophercloud.ServiceClient, error) {
+func createComputeV2FromIni(
+	ctx context.Context,
+	iniData []byte,
+	overwrite OSClientOverwrite,
+	timeout time.Duration,
+) (*gophercloud.ServiceClient, error) {
 	provider, opts, err := getProvider(ctx, iniData, overwrite, timeout)
 	if err != nil {
 		return nil, err

internal/openstack/testing/client.go

@@ -39,7 +39,7 @@ type MockClient struct {
 	ServerGroupClientObj openstack.ServerGroupClient
 }
 
-func (r *MockClient) Configure(ini []byte, overwrite openstack.OSClientOverwrite, timeout time.Duration, promCounter *prometheus.CounterVec) error {
+func (r *MockClient) Configure(_ []byte, _ openstack.OSClientOverwrite, _ time.Duration, _ *prometheus.CounterVec) error {
 	r.StoredValues = make(map[string]interface{})
 	return nil
 }