custom-cert: bugfix

api/v1alpha1/application_types.go

@@ -431,17 +431,16 @@ func (a *Application) GetCommonSpec() *CommonSpec {
 	}
 }
 
-func (s *ApplicationSpec) Hosts() ([]Host, error) {
-	var hosts []Host
+func (s *ApplicationSpec) Hosts() (HostCollection, error) {
+	hosts := NewCollection()
+
 	var errorsFound []error
 	for _, ingress := range s.Ingresses {
-		h, err := NewHost(ingress)
+		err := hosts.Add(ingress)
 		if err != nil {
 			errorsFound = append(errorsFound, err)
 			continue
 		}
-
-		hosts = append(hosts, *h)
 	}
 
 	return hosts, errors.Join(errorsFound...)

api/v1alpha1/hosts.go

@@ -15,6 +15,10 @@ type Host struct {
 	CustomCertificateSecret *string
 }
 
+type HostCollection struct {
+	hosts map[string]*Host
+}
+
 func NewHost(hostname string) (*Host, error) {
 	if len(hostname) == 0 {
 		return nil, fmt.Errorf("hostname cannot be empty")
@@ -52,3 +56,52 @@ func NewHost(hostname string) (*Host, error) {
 func (h *Host) UsesCustomCert() bool {
 	return h.CustomCertificateSecret != nil
 }
+
+func NewCollection() HostCollection {
+	return HostCollection{
+		hosts: map[string]*Host{},
+	}
+}
+
+func (hs *HostCollection) Add(hostname string) error {
+	h, err := NewHost(hostname)
+	if err != nil {
+		return err
+	}
+
+	existingValue, alreadyPresent := hs.hosts[h.Hostname]
+
+	switch alreadyPresent {
+	case true:
+		if existingValue.UsesCustomCert() {
+			return fmt.Errorf("host '%s' is already defined and using a custom certificate", existingValue.Hostname)
+		}
+		fallthrough
+	case false:
+		fallthrough
+	default:
+		hs.hosts[h.Hostname] = h
+	}
+
+	return nil
+}
+
+func (hs *HostCollection) AllHosts() []*Host {
+	hosts := make([]*Host, 0, len(hs.hosts))
+	for _, host := range hs.hosts {
+		hosts = append(hosts, host)
+	}
+	return hosts
+}
+
+func (hs *HostCollection) Hostnames() []string {
+	hostnames := make([]string, 0, len(hs.hosts))
+	for hostname := range hs.hosts {
+		hostnames = append(hostnames, hostname)
+	}
+	return hostnames
+}
+
+func (hs *HostCollection) Count() int {
+	return len(hs.hosts)
+}

internal/controllers/application.go

@@ -370,7 +370,7 @@ func validateIngresses(application *skiperatorv1alpha1.Application) error {
 	}
 
 	// TODO: Remove/rewrite?
-	for _, h := range hosts {
+	for _, h := range hosts.AllHosts() {
 		if !hostMatchExpression.MatchString(h.Hostname) {
 			errMessage := fmt.Sprintf("ingress with value '%s' was not valid. ingress must be lower case, contain no spaces, be a non-empty string, and have a hostname/domain separated by a period", h.Hostname)
 			return errors.NewInvalid(application.GroupVersionKind().GroupKind(), application.Name, field.ErrorList{

pkg/resourcegenerator/certificate/application.go

@@ -34,7 +34,7 @@ func generateForApplication(r reconciliation.Reconciliation) error {
 	}
 
 	// Generate separate cert for each ingress
-	for _, h := range hosts {
+	for _, h := range hosts.AllHosts() {
 		if h.UsesCustomCert() {
 			continue
 		}

pkg/resourcegenerator/idporten/idporten.go

@@ -76,14 +76,20 @@ func getIDPortenSpec(application *skiperatorv1alpha1.Application) (naisiov1.IDPo
 	}
 
 	ingress := KVBaseURL
-	if len(application.Spec.Ingresses) != 0 {
-		ingress = application.Spec.Ingresses[0]
+	hosts, err := application.Spec.Hosts()
+	if err != nil {
+		return naisiov1.IDPortenClientSpec{}, err
+	}
+
+	ingresses := hosts.Hostnames()
+	if len(ingresses) != 0 {
+		ingress = ingresses[0]
 	}
 	ingress = util.EnsurePrefix(ingress, "https://")
 
 	scopes := getScopes(integrationType, application.Spec.IDPorten.Scopes)
 
-	redirectURIs, err := buildURIs(application.Spec.Ingresses, application.Spec.IDPorten.RedirectPath, DefaultClientCallbackPath)
+	redirectURIs, err := buildURIs(ingresses, application.Spec.IDPorten.RedirectPath, DefaultClientCallbackPath)
 	if err != nil {
 		return naisiov1.IDPortenClientSpec{}, nil
 	}

pkg/resourcegenerator/istio/gateway/application.go

@@ -34,7 +34,7 @@ func generateForApplication(r reconciliation.Reconciliation) error {
 	}
 
 	// Generate separate gateway for each ingress
-	for _, h := range hosts {
+	for _, h := range hosts.AllHosts() {
 		name := fmt.Sprintf("%s-ingress-%x", application.Name, util.GenerateHashFromName(h.Hostname))
 		gateway := networkingv1beta1.Gateway{ObjectMeta: metav1.ObjectMeta{Namespace: application.Namespace, Name: name}}
 

pkg/resourcegenerator/istio/virtualservice/application.go

@@ -30,11 +30,17 @@ func generateForApplication(r reconciliation.Reconciliation) error {
 			Namespace: application.Namespace,
 		},
 	}
-	if len(application.Spec.Ingresses) > 0 {
+
+	hosts, err := application.Spec.Hosts()
+	if err != nil {
+		return err
+	}
+
+	if len(hosts.Hostnames()) > 0 {
 		virtualService.Spec = networkingv1beta1api.VirtualService{
 			ExportTo: []string{".", "istio-system", "istio-gateways"},
 			Gateways: getGatewaysFromApplication(application),
-			Hosts:    application.Spec.Ingresses,
+			Hosts:    hosts.Hostnames(),
 			Http:     []*networkingv1beta1api.HTTPRoute{},
 		}
 
@@ -82,8 +88,9 @@ func generateForApplication(r reconciliation.Reconciliation) error {
 }
 
 func getGatewaysFromApplication(application *skiperatorv1alpha1.Application) []string {
-	gateways := make([]string, 0, len(application.Spec.Ingresses))
-	for _, hostname := range application.Spec.Ingresses {
+	hosts, _ := application.Spec.Hosts()
+	gateways := make([]string, 0, hosts.Count())
+	for _, hostname := range hosts.Hostnames() {
 		// Generate gateway name
 		hash := fnv.New64()
 		_, _ = hash.Write([]byte(hostname))

tests/application/custom-certificate/application-duplicate-ingress-assert.yaml

@@ -0,0 +1,137 @@
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: custom-cert-duplicate
+  annotations:
+    argocd.argoproj.io/sync-options: "Prune=false"
+spec:
+  selector:
+    matchLabels:
+      app: custom-cert-duplicate
+  template:
+    metadata:
+      annotations:
+        prometheus.io/scrape: "true"
+        argocd.argoproj.io/sync-options: "Prune=false"
+        cluster-autoscaler.kubernetes.io/safe-to-evict: "true"
+      labels:
+        app: custom-cert-duplicate
+    spec:
+      containers:
+        - name: custom-cert-duplicate
+          image: image
+          imagePullPolicy: Always
+          ports:
+            - containerPort: 8080
+              protocol: TCP
+          securityContext:
+            allowPrivilegeEscalation: false
+            privileged: false
+            readOnlyRootFilesystem: true
+            runAsGroup: 150
+            runAsUser: 150
+            runAsNonRoot: true
+            capabilities:
+              drop:
+                - ALL
+              add:
+                - NET_BIND_SERVICE
+          volumeMounts:
+            - mountPath: /tmp
+              name: tmp
+      imagePullSecrets:
+        - name: github-auth
+      securityContext:
+        fsGroup: 150
+        supplementalGroups:
+          - 150
+        seccompProfile:
+          type: RuntimeDefault
+      serviceAccountName: custom-cert-duplicate
+      volumes:
+        - emptyDir: {}
+          name: tmp
+      topologySpreadConstraints:
+        - maxSkew: 1
+          topologyKey: "kubernetes.io/hostname"
+          whenUnsatisfiable: ScheduleAnyway
+          labelSelector:
+            matchExpressions:
+              - key: app
+                operator: In
+                values:
+                  - custom-cert-duplicate
+          matchLabelKeys:
+            - pod-template-hash
+        - maxSkew: 1
+          topologyKey: "onprem.gke.io/failure-domain-name"
+          whenUnsatisfiable: ScheduleAnyway
+          labelSelector:
+            matchExpressions:
+              - key: app
+                operator: In
+                values:
+                  - custom-cert-duplicate
+          matchLabelKeys:
+            - pod-template-hash
+---
+apiVersion: v1
+kind: Secret
+metadata:
+  name: some-cert
+  namespace: istio-gateways
+type: kubernetes.io/tls
+---
+apiVersion: networking.istio.io/v1beta1
+kind: Gateway
+metadata:
+  name: custom-cert-duplicate-ingress-dc2b250f77a411ad
+spec:
+  selector:
+    app: istio-ingress-external
+  servers:
+    - hosts:
+        - test.kartverket.no
+      port:
+        name: http
+        number: 80
+        protocol: HTTP
+    - hosts:
+        - test.kartverket.no
+      port:
+        name: https
+        number: 443
+        protocol: HTTPS
+      tls:
+        credentialName: some-cert
+        mode: SIMPLE
+---
+apiVersion: networking.istio.io/v1beta1
+kind: VirtualService
+metadata:
+  name: custom-cert-duplicate-ingress
+spec:
+  exportTo:
+    - .
+    - istio-system
+    - istio-gateways
+  gateways:
+    - custom-cert-duplicate-ingress-dc2b250f77a411ad
+  hosts:
+    - test.kartverket.no
+  http:
+    - match:
+        - port: 80
+          withoutHeaders:
+            ':path':
+              prefix: /.well-known/acme-challenge/
+      name: redirect-to-https
+      redirect:
+        redirectCode: 308
+        scheme: https
+    - name: default-app-route
+      route:
+        - destination:
+            host: custom-cert-duplicate
+            port:
+              number: 8080

tests/application/custom-certificate/application-duplicate-ingress-error.yaml

@@ -0,0 +1,53 @@
+apiVersion: networking.istio.io/v1beta1
+kind: Gateway
+metadata:
+  name: custom-cert-duplicate-ingress-db284ad1b14a59a0
+spec:
+  selector:
+    app: istio-ingress-external
+  servers:
+    - hosts:
+        - "test.kartverket.no+custom-cert"
+      port:
+        name: http
+        number: 80
+        protocol: HTTP
+    - hosts:
+        - "test.kartverket.no+custom-cert"
+      port:
+        name: https
+        number: 443
+        protocol: HTTPS
+      tls:
+        credentialName: some-cert
+        mode: SIMPLE
+---
+apiVersion: networking.istio.io/v1beta1
+kind: VirtualService
+metadata:
+  name: custom-cert-duplicate-ingress
+spec:
+  exportTo:
+    - .
+    - istio-system
+    - istio-gateways
+  gateways:
+    - custom-cert-duplicate-ingress-db284ad1b14a59a0
+  hosts:
+    - "test.kartverket.no+custom-cert"
+  http:
+    - match:
+        - port: 80
+          withoutHeaders:
+            ':path':
+              prefix: /.well-known/acme-challenge/
+      name: redirect-to-https
+      redirect:
+        redirectCode: 308
+        scheme: https
+    - name: default-app-route
+      route:
+        - destination:
+            host: custom-cert-duplicate
+            port:
+              number: 8080

tests/application/custom-certificate/application-duplicate-ingress.yaml

@@ -0,0 +1,10 @@
+apiVersion: skiperator.kartverket.no/v1alpha1
+kind: Application
+metadata:
+  name: custom-cert-duplicate
+spec:
+  image: image
+  port: 8080
+  ingresses:
+    - test.kartverket.no
+    - test.kartverket.no+some-cert

tests/application/custom-certificate/chainsaw-test.yaml

@@ -15,3 +15,9 @@ spec:
         - error:
             template: true
             file: generated-cert.yaml
+        - create:
+            file: application-duplicate-ingress.yaml
+        - assert:
+            file: application-duplicate-ingress-assert.yaml
+        - error:
+            file: application-duplicate-ingress-error.yaml