diff --git a/api/v1alpha1/application_types.go b/api/v1alpha1/application_types.go index e3bd00ba..6d1a9ee6 100644 --- a/api/v1alpha1/application_types.go +++ b/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...) diff --git a/api/v1alpha1/hosts.go b/api/v1alpha1/hosts.go index 9520dfef..15cc531e 100644 --- a/api/v1alpha1/hosts.go +++ b/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) +} diff --git a/internal/controllers/application.go b/internal/controllers/application.go index 2b2a8e47..ec600401 100644 --- a/internal/controllers/application.go +++ b/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{ diff --git a/pkg/resourcegenerator/certificate/application.go b/pkg/resourcegenerator/certificate/application.go index 10d585da..357f5a84 100644 --- a/pkg/resourcegenerator/certificate/application.go +++ b/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 } diff --git a/pkg/resourcegenerator/idporten/idporten.go b/pkg/resourcegenerator/idporten/idporten.go index 949b6710..cff0d91f 100644 --- a/pkg/resourcegenerator/idporten/idporten.go +++ b/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 } diff --git a/pkg/resourcegenerator/istio/gateway/application.go b/pkg/resourcegenerator/istio/gateway/application.go index 85d44823..51460698 100644 --- a/pkg/resourcegenerator/istio/gateway/application.go +++ b/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}} diff --git a/pkg/resourcegenerator/istio/virtualservice/application.go b/pkg/resourcegenerator/istio/virtualservice/application.go index 9f3dbfd0..dd8e5fda 100644 --- a/pkg/resourcegenerator/istio/virtualservice/application.go +++ b/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)) diff --git a/tests/application/custom-certificate/application-duplicate-ingress-assert.yaml b/tests/application/custom-certificate/application-duplicate-ingress-assert.yaml new file mode 100644 index 00000000..327b1fc6 --- /dev/null +++ b/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 diff --git a/tests/application/custom-certificate/application-duplicate-ingress-error.yaml b/tests/application/custom-certificate/application-duplicate-ingress-error.yaml new file mode 100644 index 00000000..682ecdfe --- /dev/null +++ b/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 diff --git a/tests/application/custom-certificate/application-duplicate-ingress.yaml b/tests/application/custom-certificate/application-duplicate-ingress.yaml new file mode 100644 index 00000000..7815d0ff --- /dev/null +++ b/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 diff --git a/tests/application/custom-certificate/chainsaw-test.yaml b/tests/application/custom-certificate/chainsaw-test.yaml index 372b5f64..4c865fde 100644 --- a/tests/application/custom-certificate/chainsaw-test.yaml +++ b/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