Merge pull request openstack-k8s-operators#558 from stuggi/18.0.2_fixes

[18.0.0-proposed] several back ports of fixes

modules/certmanager/certificate.go

@@ -19,11 +19,13 @@ package certmanager
 import (
 	"context"
 	"fmt"
+	"sort"
 	"time"
 
 	certmgrv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
 	certmgrmetav1 "github.com/cert-manager/cert-manager/pkg/apis/meta/v1"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/helper"
+	"github.com/openstack-k8s-operators/lib-common/modules/common/net"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/secret"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/service"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/util"
@@ -66,10 +68,15 @@ func NewCertificate(
 	certificate *certmgrv1.Certificate,
 	timeout time.Duration,
 ) *Certificate {
-	return &Certificate{
+	crt := &Certificate{
 		certificate: certificate,
 		timeout:     timeout,
 	}
+
+	crt.certificate.Spec.IPAddresses = net.SortIPs(crt.certificate.Spec.IPAddresses)
+	sort.Strings(crt.certificate.Spec.DNSNames)
+
+	return crt
 }
 
 // Cert returns an initialized certificate request obj.

modules/certmanager/test/functional/certmanager_test.go

@@ -166,6 +166,66 @@ var _ = Describe("certmanager module", func() {
 		Expect(cert.Labels["f"]).To(Equal("l"))
 	})
 
+	It("creates certificate with orderdered DNSNames", func() {
+		c := certmanager.NewCertificate(
+			certmanager.Cert(
+				names.CertName.Name,
+				names.CertName.Namespace,
+				map[string]string{"f": "l"},
+				certmgrv1.CertificateSpec{
+					CommonName: "keystone-public-openstack.apps-crc.testing",
+					DNSNames: []string{
+						"keystone-public-openstack.apps-crc.testing",
+						"keystone-public-openstack",
+					},
+					IssuerRef: certmgrmetav1.ObjectReference{
+						Kind: "Issuer",
+						Name: "issuerName",
+					},
+					SecretName: "secret",
+				},
+			),
+			timeout,
+		)
+
+		_, _, err := c.CreateOrPatch(ctx, h, nil)
+		Expect(err).ShouldNot(HaveOccurred())
+		cert := th.GetCert(names.CertName)
+		Expect(cert.Spec.DNSNames[0]).To(Equal("keystone-public-openstack"))
+		Expect(cert.Spec.DNSNames[1]).To(Equal("keystone-public-openstack.apps-crc.testing"))
+	})
+
+	It("creates certificate with orderdered IPAddresses", func() {
+		c := certmanager.NewCertificate(
+			certmanager.Cert(
+				names.CertName.Name,
+				names.CertName.Namespace,
+				map[string]string{"f": "l"},
+				certmgrv1.CertificateSpec{
+					CommonName: "keystone-public-openstack.apps-crc.testing",
+					IPAddresses: []string{
+						"2.2.2.2",
+						"1.1.1.1",
+						"2.2.2.1",
+					},
+					IssuerRef: certmgrmetav1.ObjectReference{
+						Kind: "Issuer",
+						Name: "issuerName",
+					},
+					SecretName: "secret",
+				},
+			),
+			timeout,
+		)
+
+		_, _, err := c.CreateOrPatch(ctx, h, nil)
+		Expect(err).ShouldNot(HaveOccurred())
+		cert := th.GetCert(names.CertName)
+		Expect(cert.Spec.IPAddresses[0]).To(Equal("1.1.1.1"))
+		Expect(cert.Spec.IPAddresses[1]).To(Equal("2.2.2.1"))
+		Expect(cert.Spec.IPAddresses[2]).To(Equal("2.2.2.2"))
+	})
+
 	It("deletes certificate", func() {
 		c := certmanager.NewCertificate(
 			certmanager.Cert(

modules/common/condition/conditions.go

@@ -318,6 +318,12 @@ const (
 	// AnsibleEEReadyErrorMessage
 	AnsibleEEReadyErrorMessage = "AnsibleEE error occurred %s"
 
+	//
+	// TLSInputReady condition messages
+	//
+	// TLSInputReadyWaitingMessage - Provides the message to clarify that TLS resources have not been generated yet
+	TLSInputReadyWaitingMessage = "TLSInput is missing: %s"
+
 	// TLSInputErrorMessage - Provides the message when there's error in provision of TLS sources
 	TLSInputErrorMessage = "TLSInput error occured in TLS sources %s"
 )

modules/common/condition/funcs.go

@@ -337,7 +337,7 @@ func (conditions *Conditions) Mirror(t Type) *Condition {
 	cg := g[groupOrder(*TrueCondition(ReadyCondition, "foo"))]
 	if len(cg.conditions) > 0 && cg.conditions.IsTrue(ReadyCondition) {
 		c := cg.conditions.Get(ReadyCondition)
-		mirrorCondition := TrueCondition(t, c.Message)
+		mirrorCondition := TrueCondition(t, "%s", c.Message)
 		mirrorCondition.LastTransitionTime = c.LastTransitionTime
 
 		return mirrorCondition
@@ -355,19 +355,19 @@ func (conditions *Conditions) Mirror(t Type) *Condition {
 		c := (*cl)[0]
 
 		if c.Status == corev1.ConditionTrue {
-			mirrorCondition = TrueCondition(t, c.Message)
+			mirrorCondition = TrueCondition(t, "%s", c.Message)
 			mirrorCondition.LastTransitionTime = c.LastTransitionTime
 			break
 		}
 
 		if c.Status == corev1.ConditionFalse {
-			mirrorCondition = FalseCondition(t, c.Reason, c.Severity, c.Message)
+			mirrorCondition = FalseCondition(t, c.Reason, c.Severity, "%s", c.Message)
 			mirrorCondition.LastTransitionTime = c.LastTransitionTime
 			break
 		}
 
 		if c.Status == corev1.ConditionUnknown {
-			mirrorCondition = UnknownCondition(t, c.Reason, c.Message)
+			mirrorCondition = UnknownCondition(t, c.Reason, "%s", c.Message)
 			mirrorCondition.LastTransitionTime = c.LastTransitionTime
 			break
 		}

modules/common/daemonset/daemonset.go

@@ -83,7 +83,7 @@ func (d *DaemonSet) CreateOrPatch(
 	}
 
 	// update the daemonset object of the daemonset type
-	d.daemonset, err = getDaemonSetWithName(ctx, h, daemonset.GetName(), daemonset.GetNamespace())
+	d.daemonset, err = GetDaemonSetWithName(ctx, h, daemonset.GetName(), daemonset.GetNamespace())
 	if err != nil {
 		if k8s_errors.IsNotFound(err) {
 			return ctrl.Result{RequeueAfter: d.timeout}, nil
@@ -112,7 +112,8 @@ func (d *DaemonSet) GetDaemonSet() appsv1.DaemonSet {
 	return *d.daemonset
 }
 
-func getDaemonSetWithName(
+// GetDaemonSetWithName - get the daemonset object with a given name.
+func GetDaemonSetWithName(
 	ctx context.Context,
 	h *helper.Helper,
 	name string,

modules/common/net/ip.go

@@ -0,0 +1,46 @@
+/*
+Copyright 2023 Red Hat
+
+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 net
+
+import (
+	"bytes"
+	"net"
+	"sort"
+)
+
+// SortIPs - Get network-attachment-definition with name in namespace
+func SortIPs(
+	ips []string,
+) []string {
+	netIPs := make([]net.IP, 0, len(ips))
+
+	for _, ip := range ips {
+		netIPs = append(netIPs, net.ParseIP(ip))
+	}
+
+	sort.Slice(netIPs, func(i, j int) bool {
+		return bytes.Compare(netIPs[i], netIPs[j]) < 0
+	})
+
+	sortedIPs := make([]string, 0, len(netIPs))
+
+	for _, ip := range netIPs {
+		sortedIPs = append(sortedIPs, ip.String())
+	}
+
+	return sortedIPs
+}

modules/common/net/ip_test.go

@@ -0,0 +1,94 @@
+/*
+Copyright 2023 Red Hat
+
+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 net
+
+import (
+	"testing"
+
+	. "github.com/onsi/gomega"
+)
+
+func TestSortIPs(t *testing.T) {
+
+	tests := []struct {
+		name string
+		ips  []string
+		want []string
+	}{
+		{
+			name: "empty ip list",
+			ips:  []string{},
+			want: []string{},
+		},
+		{
+			name: "IPv4 - single ip in list",
+			ips:  []string{"1.1.1.1"},
+			want: []string{"1.1.1.1"},
+		},
+		{
+			name: "IPv4 - already sorted list",
+			ips:  []string{"1.1.1.1", "2.2.2.2"},
+			want: []string{"1.1.1.1", "2.2.2.2"},
+		},
+		{
+			name: "IPv4 - unsorted sorted list",
+			ips:  []string{"2.2.2.2", "1.1.1.1"},
+			want: []string{"1.1.1.1", "2.2.2.2"},
+		},
+		{
+			name: "IPv4 - another unsorted sorted list",
+			ips:  []string{"2.2.2.2", "1.1.1.2", "1.1.1.1"},
+			want: []string{"1.1.1.1", "1.1.1.2", "2.2.2.2"},
+		},
+		{
+			name: "IPv6 - single ip in list",
+			ips:  []string{"fd00:bbbb::1"},
+			want: []string{"fd00:bbbb::1"},
+		},
+		{
+			name: "IPv6 - already sorted list",
+			ips:  []string{"fd00:bbbb::1", "fd00:bbbb::2"},
+			want: []string{"fd00:bbbb::1", "fd00:bbbb::2"},
+		},
+		{
+			name: "IPv6 - unsorted sorted list",
+			ips:  []string{"fd00:bbbb::2", "fd00:bbbb::1"},
+			want: []string{"fd00:bbbb::1", "fd00:bbbb::2"},
+		},
+		{
+			name: "IPv6 - another unsorted sorted list",
+			ips:  []string{"fd00:bbbb::2", "fd00:aaaa::1", "fd00:bbbb::1"},
+			want: []string{"fd00:aaaa::1", "fd00:bbbb::1", "fd00:bbbb::2"},
+		},
+		{
+			name: "IPV4 and IPv6 - unsorted sorted list",
+			ips:  []string{"fd00:bbbb::2", "fd00:aaaa::1", "fd00:bbbb::1", "1.1.1.1"},
+			want: []string{"1.1.1.1", "fd00:aaaa::1", "fd00:bbbb::1", "fd00:bbbb::2"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			sortedIPs := SortIPs(tt.ips)
+			g.Expect(sortedIPs).NotTo(BeNil())
+			g.Expect(sortedIPs).To(HaveLen(len(tt.want)))
+			g.Expect(sortedIPs).To(BeEquivalentTo(tt.want))
+		})
+	}
+}

modules/common/service/types_test.go

@@ -134,7 +134,7 @@ func TestValidateRoutedOverrides(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			g := NewWithT(t)
 
-			g.Expect(ValidateRoutedOverrides(tt.basePath, tt.overrides)).To(Equal(tt.want))
+			g.Expect(ValidateRoutedOverrides(tt.basePath, tt.overrides)).To(ContainElements(tt.want))
 		})
 	}
 }

modules/openstack/domain.go

@@ -38,7 +38,7 @@ func (o *OpenStack) CreateDomain(log logr.Logger, d Domain) (string, error) {
 		}
 		domainID = domain.ID
 	} else {
-		return domainID, fmt.Errorf(fmt.Sprintf("Multiple domains named \"%s\" found", d.Name))
+		return domainID, fmt.Errorf("Multiple domains named \"%s\" found", d.Name)
 	}
 
 	return domainID, nil

modules/openstack/limits.go

@@ -86,7 +86,7 @@ func (o *OpenStack) CreateLimit(
 		}
 		limitID = createdLimits[0].ID
 	} else {
-		return limitID, fmt.Errorf(fmt.Sprintf("multiple limits named \"%s\" found", l.ResourceName))
+		return limitID, fmt.Errorf("multiple limits named \"%s\" found", l.ResourceName)
 	}
 
 	return limitID, nil
@@ -156,7 +156,7 @@ func (o *OpenStack) CreateOrUpdateRegisteredLimit(
 		}
 		limitID = createdLimits[0].ID
 	} else {
-		return limitID, fmt.Errorf(fmt.Sprintf("multiple limits named \"%s\" found", l.ResourceName))
+		return limitID, fmt.Errorf("multiple limits named \"%s\" found", l.ResourceName)
 	}
 
 	return limitID, nil

modules/openstack/project.go

@@ -62,7 +62,7 @@ func (o *OpenStack) CreateProject(
 		}
 		projectID = project.ID
 	} else {
-		return projectID, fmt.Errorf(fmt.Sprintf("multiple projects named \"%s\" found", p.Name))
+		return projectID, fmt.Errorf("multiple projects named \"%s\" found", p.Name)
 	}
 
 	return projectID, nil
@@ -84,9 +84,9 @@ func (o *OpenStack) GetProject(
 	}
 
 	if len(allProjects) == 0 {
-		return nil, fmt.Errorf(fmt.Sprintf("%s %s", projectName, ProjectNotFound))
+		return nil, fmt.Errorf("%s %s", projectName, ProjectNotFound)
 	} else if len(allProjects) > 1 {
-		return nil, fmt.Errorf(fmt.Sprintf("multiple project named \"%s\" found", projectName))
+		return nil, fmt.Errorf("multiple project named \"%s\" found", projectName)
 	}
 
 	return &allProjects[0], nil

modules/openstack/role.go

@@ -80,7 +80,7 @@ func (o *OpenStack) GetRole(
 	}
 
 	if len(allRoles) == 0 {
-		return nil, fmt.Errorf(fmt.Sprintf("%s %s", roleName, RoleNotFound))
+		return nil, fmt.Errorf("%s %s", roleName, RoleNotFound)
 	}
 
 	return &allRoles[0], nil

modules/openstack/service.go

@@ -96,7 +96,7 @@ func (o *OpenStack) GetService(
 	}
 
 	if len(allServices) == 0 {
-		return nil, fmt.Errorf(fmt.Sprintf("%s %s", serviceName, ServiceNotFound))
+		return nil, fmt.Errorf("%s %s", serviceName, ServiceNotFound)
 	}
 
 	return &allServices[0], nil

modules/openstack/user.go

@@ -94,9 +94,9 @@ func (o *OpenStack) GetUser(
 	}
 
 	if len(allUsers) == 0 {
-		return nil, fmt.Errorf(fmt.Sprintf("%s %s", userName, UserNotFound))
+		return nil, fmt.Errorf("%s %s", userName, UserNotFound)
 	} else if len(allUsers) > 1 {
-		return nil, fmt.Errorf(fmt.Sprintf("multiple users named \"%s\" found", userName))
+		return nil, fmt.Errorf("multiple users named \"%s\" found", userName)
 	}
 
 	return &allUsers[0], nil