From d10ed546ace2ffafb8b9f5c7f8044b6ddc6796ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A5rd=20Ove=20Hoel?= Date: Mon, 19 Aug 2024 10:58:30 +0200 Subject: [PATCH 01/34] Support for multiple types in ingresses --- api/v1alpha1/application_types.go | 57 ++++++++++++++++--- api/v1alpha1/zz_generated.deepcopy.go | 4 +- ...skiperator.kartverket.no_applications.yaml | 4 +- .../deployment/deployment.go | 10 +++- .../networkpolicy/dynamic/common.go | 10 +++- 5 files changed, 71 insertions(+), 14 deletions(-) diff --git a/api/v1alpha1/application_types.go b/api/v1alpha1/application_types.go index b2fd31ac..7afd3b99 100644 --- a/api/v1alpha1/application_types.go +++ b/api/v1alpha1/application_types.go @@ -3,13 +3,14 @@ package v1alpha1 import ( "encoding/json" "errors" + "fmt" + "time" "github.com/kartverket/skiperator/api/v1alpha1/digdirator" "github.com/kartverket/skiperator/api/v1alpha1/podtypes" corev1 "k8s.io/api/core/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" - "time" ) // +kubebuilder:object:root=true @@ -67,7 +68,7 @@ type ApplicationSpec struct { // They can optionally be suffixed with a plus and name of a custom TLS secret located in the istio-gateways namespace. // E.g. "foo.atkv3-dev.kartverket-intern.cloud+env-wildcard-cert" //+kubebuilder:validation:Optional - Ingresses []string `json:"ingresses,omitempty"` + Ingresses *apiextensionsv1.JSON `json:"ingresses,omitempty"` // An optional priority. Supported values are 'low', 'medium' and 'high'. // The default value is 'medium'. @@ -432,19 +433,61 @@ func (a *Application) GetCommonSpec() *CommonSpec { } } +type Ingresses struct { + Hostname string + Internal bool +} + +func MarshalledIngresses(Ingresses interface{}) *apiextensionsv1.JSON { + IngressesJSON := &apiextensionsv1.JSON{} + var err error + + IngressesJSON.Raw, err = json.Marshal(Ingresses) + if err == nil { + return IngressesJSON + } + + return nil +} + func (s *ApplicationSpec) Hosts() (HostCollection, error) { + var resultString []string + var resultObject []Ingresses + var errorsFound []error + hosts := NewCollection() - var errorsFound []error - for _, ingress := range s.Ingresses { + ingresses := MarshalledIngresses(s.Ingresses) + err := json.Unmarshal(ingresses.Raw, &resultString) + + + if err != nil { + err := json.Unmarshal(ingresses.Raw, &resultObject) + if err != nil { + errorsFound = append(errorsFound, err) + return HostCollection{}, errors.Join(errorsFound...) + } + + + for _, ingress := range resultObject { + fmt.Printf("############################### %v\n", ingress.Hostname) + err := hosts.Add(ingress.Hostname) + if err != nil { + errorsFound = append(errorsFound, err) + } + } + return HostCollection{}, nil + } + + for _, ingress := range resultString { + fmt.Printf("############################### %v\n", ingress) err := hosts.Add(ingress) if err != nil { errorsFound = append(errorsFound, err) - continue } } - - return hosts, errors.Join(errorsFound...) + + return HostCollection{}, nil } type MultiErr interface { diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index a41dd7e9..7f454bef 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -77,8 +77,8 @@ func (in *ApplicationSpec) DeepCopyInto(out *ApplicationSpec) { *out = *in if in.Ingresses != nil { in, out := &in.Ingresses, &out.Ingresses - *out = make([]string, len(*in)) - copy(*out, *in) + *out = new(v1.JSON) + (*in).DeepCopyInto(*out) } if in.Command != nil { in, out := &in.Command, &out.Command diff --git a/config/crd/skiperator.kartverket.no_applications.yaml b/config/crd/skiperator.kartverket.no_applications.yaml index 3402ae35..74b87d6f 100644 --- a/config/crd/skiperator.kartverket.no_applications.yaml +++ b/config/crd/skiperator.kartverket.no_applications.yaml @@ -646,9 +646,7 @@ spec: Ingresses must be lowercase, contain no spaces, be a non-empty string, and have a hostname/domain separated by a period They can optionally be suffixed with a plus and name of a custom TLS secret located in the istio-gateways namespace. E.g. "foo.atkv3-dev.kartverket-intern.cloud+env-wildcard-cert" - items: - type: string - type: array + x-kubernetes-preserve-unknown-fields: true labels: additionalProperties: type: string diff --git a/pkg/resourcegenerator/deployment/deployment.go b/pkg/resourcegenerator/deployment/deployment.go index bb0d6199..bc0b8e45 100644 --- a/pkg/resourcegenerator/deployment/deployment.go +++ b/pkg/resourcegenerator/deployment/deployment.go @@ -204,10 +204,18 @@ func Generate(r reconciliation.Reconciliation) error { } // add an external link to argocd - ingresses := application.Spec.Ingresses + hosts, err := application.Spec.Hosts() + if err != nil { + ctxLog.Error(err, "could not get hosts from application") + } if deployment.Annotations == nil { deployment.Annotations = make(map[string]string) } + + var ingresses []string + for _, host := range hosts.AllHosts() { + ingresses = append(ingresses, host.Hostname) + } if len(ingresses) > 0 { deployment.Annotations[AnnotationKeyLinkPrefix] = fmt.Sprintf("https://%s", ingresses[0]) diff --git a/pkg/resourcegenerator/networkpolicy/dynamic/common.go b/pkg/resourcegenerator/networkpolicy/dynamic/common.go index 50f1fe5c..6ae2c080 100644 --- a/pkg/resourcegenerator/networkpolicy/dynamic/common.go +++ b/pkg/resourcegenerator/networkpolicy/dynamic/common.go @@ -37,8 +37,16 @@ func generateForCommon(r reconciliation.Reconciliation) error { accessPolicy := object.GetCommonSpec().AccessPolicy var ingresses []string var inboundPort int32 + + hosts, err := object.(*skiperatorv1alpha1.Application).Spec.Hosts() + if err != nil { + ctxLog.Error(err, "Failed to get hosts for networkpolicy", "type", r.GetType(), "namespace", namespace) + } + + for _, host := range hosts.AllHosts() { + ingresses = append(ingresses, host.Hostname) + } if r.GetType() == reconciliation.ApplicationType { - ingresses = object.(*skiperatorv1alpha1.Application).Spec.Ingresses inboundPort = int32(object.(*skiperatorv1alpha1.Application).Spec.Port) } From 4d206e3150346655c55fa4ea38e93b7bc63d2a9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A5rd=20Ove=20Hoel?= Date: Tue, 3 Sep 2024 08:53:10 +0200 Subject: [PATCH 02/34] changes --- api/v1alpha1/application_types.go | 27 ++++------ api/v1alpha1/hosts.go | 33 ++++++++++++ .../deployment/deployment.go | 7 +-- .../istio/gateway/application.go | 3 +- .../istio/gateway/routing.go | 2 +- .../networkpolicy/dynamic/common.go | 53 +++++-------------- .../networkpolicy/dynamic/routing.go | 7 ++- pkg/util/helperfunctions.go | 30 +++++++---- ...application-is-internal-vanity-assert.yaml | 46 ++++++++++++++++ .../application-is-internal-vanity.yaml | 10 ++++ tests/application/ingress/chainsaw-test.yaml | 5 ++ 11 files changed, 147 insertions(+), 76 deletions(-) create mode 100644 tests/application/ingress/application-is-internal-vanity-assert.yaml create mode 100644 tests/application/ingress/application-is-internal-vanity.yaml diff --git a/api/v1alpha1/application_types.go b/api/v1alpha1/application_types.go index 7afd3b99..9c8fede7 100644 --- a/api/v1alpha1/application_types.go +++ b/api/v1alpha1/application_types.go @@ -3,14 +3,13 @@ package v1alpha1 import ( "encoding/json" "errors" - "fmt" - "time" "github.com/kartverket/skiperator/api/v1alpha1/digdirator" "github.com/kartverket/skiperator/api/v1alpha1/podtypes" corev1 "k8s.io/api/core/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" + "time" ) // +kubebuilder:object:root=true @@ -454,40 +453,36 @@ func (s *ApplicationSpec) Hosts() (HostCollection, error) { var resultString []string var resultObject []Ingresses var errorsFound []error - hosts := NewCollection() ingresses := MarshalledIngresses(s.Ingresses) err := json.Unmarshal(ingresses.Raw, &resultString) - - + if err != nil { err := json.Unmarshal(ingresses.Raw, &resultObject) if err != nil { errorsFound = append(errorsFound, err) - return HostCollection{}, errors.Join(errorsFound...) + return NewCollection(), errors.Join(errorsFound...) } - - + for _, ingress := range resultObject { - fmt.Printf("############################### %v\n", ingress.Hostname) - err := hosts.Add(ingress.Hostname) + err := hosts.AddObject(ingress.Hostname, ingress.Internal) if err != nil { errorsFound = append(errorsFound, err) + return NewCollection(), errors.Join(errorsFound...) } } - return HostCollection{}, nil + return hosts, nil } - + for _, ingress := range resultString { - fmt.Printf("############################### %v\n", ingress) - err := hosts.Add(ingress) + err := hosts.AddObject(ingress, IsInternal(ingress)) if err != nil { errorsFound = append(errorsFound, err) } } - - return HostCollection{}, nil + + return hosts, errors.Join(errorsFound...) } type MultiErr interface { diff --git a/api/v1alpha1/hosts.go b/api/v1alpha1/hosts.go index 15cc531e..cd56bd98 100644 --- a/api/v1alpha1/hosts.go +++ b/api/v1alpha1/hosts.go @@ -3,16 +3,20 @@ package v1alpha1 import ( "fmt" "github.com/chmike/domain" + "regexp" "strings" ) const hostnameSecretSeparator = "+" +var internalPattern = regexp.MustCompile(`[^.]\.skip\.statkart\.no|[^.]\.kartverket-intern.cloud`) + // TODO: Add a mechanism for validating that the // hostname is covered by the CustomCertificateSecret if present type Host struct { Hostname string CustomCertificateSecret *string + Internal bool } type HostCollection struct { @@ -25,6 +29,9 @@ func NewHost(hostname string) (*Host, error) { } var h Host + + h.Internal = IsInternal(hostname) + // If hostname is separated by +, the user wants to use a custom certificate results := strings.Split(hostname, hostnameSecretSeparator) @@ -70,7 +77,29 @@ func (hs *HostCollection) Add(hostname string) error { } 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) AddObject(hostname string, internal bool) error { + h, err := NewHost(hostname) + if err != nil { + return err + } + h.Internal = internal + existingValue, alreadyPresent := hs.hosts[h.Hostname] switch alreadyPresent { case true: if existingValue.UsesCustomCert() { @@ -105,3 +134,7 @@ func (hs *HostCollection) Hostnames() []string { func (hs *HostCollection) Count() int { return len(hs.hosts) } + +func IsInternal(hostname string) bool { + return internalPattern.MatchString(hostname) +} diff --git a/pkg/resourcegenerator/deployment/deployment.go b/pkg/resourcegenerator/deployment/deployment.go index bc0b8e45..e3a2cc9c 100644 --- a/pkg/resourcegenerator/deployment/deployment.go +++ b/pkg/resourcegenerator/deployment/deployment.go @@ -211,11 +211,8 @@ func Generate(r reconciliation.Reconciliation) error { if deployment.Annotations == nil { deployment.Annotations = make(map[string]string) } - - var ingresses []string - for _, host := range hosts.AllHosts() { - ingresses = append(ingresses, host.Hostname) - } + + var ingresses = hosts.Hostnames() if len(ingresses) > 0 { deployment.Annotations[AnnotationKeyLinkPrefix] = fmt.Sprintf("https://%s", ingresses[0]) diff --git a/pkg/resourcegenerator/istio/gateway/application.go b/pkg/resourcegenerator/istio/gateway/application.go index 51460698..39791107 100644 --- a/pkg/resourcegenerator/istio/gateway/application.go +++ b/pkg/resourcegenerator/istio/gateway/application.go @@ -2,7 +2,6 @@ package gateway import ( "fmt" - skiperatorv1alpha1 "github.com/kartverket/skiperator/api/v1alpha1" "github.com/kartverket/skiperator/pkg/reconciliation" "github.com/kartverket/skiperator/pkg/util" @@ -38,7 +37,7 @@ func generateForApplication(r reconciliation.Reconciliation) error { name := fmt.Sprintf("%s-ingress-%x", application.Name, util.GenerateHashFromName(h.Hostname)) gateway := networkingv1beta1.Gateway{ObjectMeta: metav1.ObjectMeta{Namespace: application.Namespace, Name: name}} - gateway.Spec.Selector = util.GetIstioGatewayLabelSelector(h.Hostname) + gateway.Spec.Selector = util.GetIstioGatewayLabelSelector(h) gatewayServersToAdd := []*networkingv1beta1api.Server{} diff --git a/pkg/resourcegenerator/istio/gateway/routing.go b/pkg/resourcegenerator/istio/gateway/routing.go index 714dbc87..3f08edba 100644 --- a/pkg/resourcegenerator/istio/gateway/routing.go +++ b/pkg/resourcegenerator/istio/gateway/routing.go @@ -44,7 +44,7 @@ func generateForRouting(r reconciliation.Reconciliation) error { } } - gateway.Spec.Selector = util.GetIstioGatewayLabelSelector(h.Hostname) + gateway.Spec.Selector = util.GetIstioGatewayLabelSelector(h) gateway.Spec.Servers = []*networkingv1beta1api.Server{ { Hosts: []string{h.Hostname}, diff --git a/pkg/resourcegenerator/networkpolicy/dynamic/common.go b/pkg/resourcegenerator/networkpolicy/dynamic/common.go index 6ae2c080..07d61150 100644 --- a/pkg/resourcegenerator/networkpolicy/dynamic/common.go +++ b/pkg/resourcegenerator/networkpolicy/dynamic/common.go @@ -35,22 +35,20 @@ func generateForCommon(r reconciliation.Reconciliation) error { } accessPolicy := object.GetCommonSpec().AccessPolicy - var ingresses []string + var hosts = skiperatorv1alpha1.NewCollection() var inboundPort int32 - hosts, err := object.(*skiperatorv1alpha1.Application).Spec.Hosts() - if err != nil { - ctxLog.Error(err, "Failed to get hosts for networkpolicy", "type", r.GetType(), "namespace", namespace) - } - - for _, host := range hosts.AllHosts() { - ingresses = append(ingresses, host.Hostname) - } if r.GetType() == reconciliation.ApplicationType { + var err error + hosts, err = object.(*skiperatorv1alpha1.Application).Spec.Hosts() + if err != nil { + ctxLog.Error(err, "Failed to get hosts for networkpolicy", "type", r.GetType(), "namespace", namespace) + return err + } inboundPort = int32(object.(*skiperatorv1alpha1.Application).Spec.Port) } - ingressRules := getIngressRules(accessPolicy, ingresses, r.IsIstioEnabled(), namespace, inboundPort) + ingressRules := getIngressRules(accessPolicy, hosts, r.IsIstioEnabled(), namespace, inboundPort) egressRules := getEgressRules(accessPolicy, namespace) netpolSpec := networkingv1.NetworkPolicySpec{ @@ -118,20 +116,14 @@ func getEgressRule(outboundRule podtypes.InternalRule, namespace string) network } // TODO Clean up better -func getIngressRules(accessPolicy *podtypes.AccessPolicy, ingresses []string, istioEnabled bool, namespace string, port int32) []networkingv1.NetworkPolicyIngressRule { +func getIngressRules(accessPolicy *podtypes.AccessPolicy, hostCollection skiperatorv1alpha1.HostCollection, istioEnabled bool, namespace string, port int32) []networkingv1.NetworkPolicyIngressRule { var ingressRules []networkingv1.NetworkPolicyIngressRule - if ingresses != nil && len(ingresses) > 0 { - if hasInternalIngress(ingresses) { - ingressRules = append(ingressRules, getGatewayIngressRule(true, port)) - } - - if hasExternalIngress(ingresses) { - ingressRules = append(ingressRules, getGatewayIngressRule(false, port)) - } + for _, host := range hostCollection.AllHosts() { + ingressRules = append(ingressRules, getGatewayIngressRule(host.Internal, port)) } - // Allow grafana-agent to scrape + // Allow grafana-agent and Alloy to scrape if istioEnabled { promScrapeRuleGrafana := networkingv1.NetworkPolicyIngressRule{ From: []networkingv1.NetworkPolicyPeer{ @@ -231,26 +223,6 @@ func getNamespaceSelector(rule podtypes.InternalRule, appNamespace string) *meta } } -func hasExternalIngress(ingresses []string) bool { - for _, hostname := range ingresses { - if !util.IsInternal(hostname) { - return true - } - } - - return false -} - -func hasInternalIngress(ingresses []string) bool { - for _, hostname := range ingresses { - if util.IsInternal(hostname) { - return true - } - } - - return false -} - func getGatewayIngressRule(isInternal bool, port int32) networkingv1.NetworkPolicyIngressRule { ingressRule := networkingv1.NetworkPolicyIngressRule{ From: []networkingv1.NetworkPolicyPeer{ @@ -259,6 +231,7 @@ func getGatewayIngressRule(isInternal bool, port int32) networkingv1.NetworkPoli MatchLabels: map[string]string{"kubernetes.io/metadata.name": "istio-gateways"}, }, PodSelector: &metav1.LabelSelector{ + MatchLabels: getIngressGatewayLabel(isInternal), }, }, diff --git a/pkg/resourcegenerator/networkpolicy/dynamic/routing.go b/pkg/resourcegenerator/networkpolicy/dynamic/routing.go index 11fd658a..1a525731 100644 --- a/pkg/resourcegenerator/networkpolicy/dynamic/routing.go +++ b/pkg/resourcegenerator/networkpolicy/dynamic/routing.go @@ -27,7 +27,10 @@ func generateForRouting(r reconciliation.Reconciliation) error { for _, route := range routing.Spec.Routes { uniqueTargetApps[getNetworkPolicyName(routing, route.TargetApp)] = route } - + h, err := routing.Spec.GetHost() + if err != nil { + return err + } for netpolName, route := range uniqueTargetApps { networkPolicy := networkingv1.NetworkPolicy{ ObjectMeta: metav1.ObjectMeta{ @@ -50,7 +53,7 @@ func generateForRouting(r reconciliation.Reconciliation) error { MatchLabels: util.GetIstioGatewaySelector(), }, PodSelector: &metav1.LabelSelector{ - MatchLabels: util.GetIstioGatewayLabelSelector(routing.Spec.Hostname), + MatchLabels: util.GetIstioGatewayLabelSelector(h), }, }, }, diff --git a/pkg/util/helperfunctions.go b/pkg/util/helperfunctions.go index a3bf824e..b54e872a 100644 --- a/pkg/util/helperfunctions.go +++ b/pkg/util/helperfunctions.go @@ -3,6 +3,7 @@ package util import ( "context" "fmt" + "github.com/kartverket/skiperator/api/v1alpha1" "github.com/kartverket/skiperator/api/v1alpha1/podtypes" "github.com/mitchellh/hashstructure/v2" "github.com/nais/liberator/pkg/namegen" @@ -13,26 +14,20 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/validation" "k8s.io/client-go/tools/record" + "reflect" "regexp" "sigs.k8s.io/controller-runtime/pkg/client" "strings" "unicode" ) -//TODO Clean up this file, move functions to more appropriate files +// TODO Clean up this file, move functions to more appropriate files -var internalPattern = regexp.MustCompile(`[^.]\.skip\.statkart\.no|[^.]\.kartverket-intern.cloud`) - -func IsInternal(hostname string) bool { - return internalPattern.MatchString(hostname) -} - -func GetIstioGatewayLabelSelector(hostname string) map[string]string { - if IsInternal(hostname) { +func GetIstioGatewayLabelSelector(host *v1alpha1.Host) map[string]string { + if host.Internal { return map[string]string{"app": "istio-ingress-internal"} } return map[string]string{"app": "istio-ingress-external"} - } func GetHashForStructs(obj []interface{}) string { @@ -148,6 +143,21 @@ func EnsurePrefix(s string, prefix string) string { return s } +func GetObjectDiff[T any](a T, b T) (diff.Changelog, error) { + aKind := reflect.ValueOf(a).Kind() + bKind := reflect.ValueOf(b).Kind() + if aKind != bKind { + return nil, fmt.Errorf("The objects to compare are not the same, found obj1: %v, obj2: %v\n", aKind, bKind) + } + changelog, err := diff.Diff(a, b) + + if len(changelog) == 0 { + return nil, err + } + + return changelog, nil +} + func IsCloudSqlProxyEnabled(gcp *podtypes.GCP) bool { return gcp != nil && gcp.CloudSQLProxy != nil } diff --git a/tests/application/ingress/application-is-internal-vanity-assert.yaml b/tests/application/ingress/application-is-internal-vanity-assert.yaml new file mode 100644 index 00000000..53abe1af --- /dev/null +++ b/tests/application/ingress/application-is-internal-vanity-assert.yaml @@ -0,0 +1,46 @@ +apiVersion: networking.istio.io/v1beta1 +kind: Gateway +metadata: + name: internalvanity-ingress-dc2b250f77a411ad +spec: + selector: + app: istio-ingress-internal +--- +apiVersion: networking.k8s.io/v1 +kind: NetworkPolicy +metadata: + name: internalvanity +spec: + ingress: + - from: + - namespaceSelector: + matchLabels: + kubernetes.io/metadata.name: istio-gateways + podSelector: + matchLabels: + app: istio-ingress-internal + podSelector: + matchLabels: + app: internalvanity +--- +apiVersion: networking.k8s.io/v1 +kind: NetworkPolicy +metadata: + name: internalvanity +spec: + ingress: + - from: + - namespaceSelector: + matchLabels: + kubernetes.io/metadata.name: istio-gateways + podSelector: + matchLabels: + app: istio-ingress-internal + ports: + - port: 8080 + protocol: TCP + podSelector: + matchLabels: + app: internalvanity + policyTypes: + - Ingress diff --git a/tests/application/ingress/application-is-internal-vanity.yaml b/tests/application/ingress/application-is-internal-vanity.yaml new file mode 100644 index 00000000..fe3f59e5 --- /dev/null +++ b/tests/application/ingress/application-is-internal-vanity.yaml @@ -0,0 +1,10 @@ +apiVersion: skiperator.kartverket.no/v1alpha1 +kind: Application +metadata: + name: internalvanity +spec: + image: image + port: 8080 + ingresses: + - hostname: test.kartverket.no + internal: true \ No newline at end of file diff --git a/tests/application/ingress/chainsaw-test.yaml b/tests/application/ingress/chainsaw-test.yaml index d23d073a..7e911029 100644 --- a/tests/application/ingress/chainsaw-test.yaml +++ b/tests/application/ingress/chainsaw-test.yaml @@ -55,3 +55,8 @@ spec: file: application-is-external.yaml - assert: file: application-is-external-assert.yaml + - try: + - create: + file: application-is-internal-vanity.yaml + - assert: + file: application-is-internal-vanity-assert.yaml From 2f605656a8a6eaf1f9d13565c59a8ce374261904 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A5rd=20Ove=20Hoel?= Date: Wed, 4 Sep 2024 10:06:58 +0200 Subject: [PATCH 03/34] Removed unused functions --- pkg/util/helperfunctions.go | 64 ------------------------------------- 1 file changed, 64 deletions(-) diff --git a/pkg/util/helperfunctions.go b/pkg/util/helperfunctions.go index b54e872a..d0939183 100644 --- a/pkg/util/helperfunctions.go +++ b/pkg/util/helperfunctions.go @@ -9,16 +9,10 @@ import ( "github.com/nais/liberator/pkg/namegen" "hash/fnv" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/validation" - "k8s.io/client-go/tools/record" - "reflect" - "regexp" "sigs.k8s.io/controller-runtime/pkg/client" "strings" - "unicode" ) // TODO Clean up this file, move functions to more appropriate files @@ -52,35 +46,6 @@ func GetConfigMap(client client.Client, ctx context.Context, namespacedName type return configMap, err } -func GetSecret(client client.Client, ctx context.Context, namespacedName types.NamespacedName) (corev1.Secret, error) { - secret := corev1.Secret{} - - err := client.Get(ctx, namespacedName, &secret) - - return secret, err -} - -func GetService(client client.Client, ctx context.Context, namespacedName types.NamespacedName) (corev1.Service, error) { - service := corev1.Service{} - - err := client.Get(ctx, namespacedName, &service) - - return service, err -} - -func ErrIsMissingOrNil(recorder record.EventRecorder, err error, message string, object runtime.Object) bool { - if errors.IsNotFound(err) { - recorder.Eventf( - object, - corev1.EventTypeWarning, "Missing", - message, - ) - } else if err != nil { - return false - } - return true -} - func ErrDoPanic(err error, message string) { if err != nil { errorMessage := fmt.Sprintf(message, err.Error()) @@ -111,24 +76,10 @@ func GetPodAppAndTeamSelector(applicationName string, teamName string) map[strin } } -func HasUpperCaseLetter(word string) bool { - for _, letter := range word { - if unicode.IsUpper(letter) { - return true - } - } - - return false -} - func ResourceNameWithKindPostfix(resourceName string, kind string) string { return strings.ToLower(fmt.Sprintf("%v-%v", resourceName, kind)) } -func GetGatewaySecretName(namespace string, name string) string { - return fmt.Sprintf("%s-%s-ingress", namespace, name) -} - func GetSecretName(prefix string, name string) (string, error) { // https://github.com/nais/naiserator/blob/faed273b68dff8541e1e2889fda5d017730f9796/pkg/resourcecreator/idporten/idporten.go#L82 // https://github.com/nais/naiserator/blob/faed273b68dff8541e1e2889fda5d017730f9796/pkg/resourcecreator/idporten/idporten.go#L170 @@ -143,21 +94,6 @@ func EnsurePrefix(s string, prefix string) string { return s } -func GetObjectDiff[T any](a T, b T) (diff.Changelog, error) { - aKind := reflect.ValueOf(a).Kind() - bKind := reflect.ValueOf(b).Kind() - if aKind != bKind { - return nil, fmt.Errorf("The objects to compare are not the same, found obj1: %v, obj2: %v\n", aKind, bKind) - } - changelog, err := diff.Diff(a, b) - - if len(changelog) == 0 { - return nil, err - } - - return changelog, nil -} - func IsCloudSqlProxyEnabled(gcp *podtypes.GCP) bool { return gcp != nil && gcp.CloudSQLProxy != nil } From 13efe89ded479573b43fc8f853e0cf2432d741f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A5rd=20Ove=20Hoel?= Date: Thu, 5 Sep 2024 09:17:32 +0200 Subject: [PATCH 04/34] tests --- tests/routing/internal-flag/application.yaml | 15 +++ .../routing/internal-flag/chainsaw-test.yaml | 18 ++++ .../routing-internal-flag-assert.yaml | 94 +++++++++++++++++++ .../internal-flag/routing-internal-flag.yaml | 15 +++ 4 files changed, 142 insertions(+) create mode 100644 tests/routing/internal-flag/application.yaml create mode 100644 tests/routing/internal-flag/chainsaw-test.yaml create mode 100644 tests/routing/internal-flag/routing-internal-flag-assert.yaml create mode 100644 tests/routing/internal-flag/routing-internal-flag.yaml diff --git a/tests/routing/internal-flag/application.yaml b/tests/routing/internal-flag/application.yaml new file mode 100644 index 00000000..32c89a0f --- /dev/null +++ b/tests/routing/internal-flag/application.yaml @@ -0,0 +1,15 @@ +apiVersion: skiperator.kartverket.no/v1alpha1 +kind: Application +metadata: + name: app-1 +spec: + image: gcr.io/google-samples/istio/helloserver:v0.0.1 + port: 8000 +--- +apiVersion: skiperator.kartverket.no/v1alpha1 +kind: Application +metadata: + name: app-2 +spec: + image: gcr.io/google-samples/istio/helloserver:v0.0.1 + port: 9000 diff --git a/tests/routing/internal-flag/chainsaw-test.yaml b/tests/routing/internal-flag/chainsaw-test.yaml new file mode 100644 index 00000000..fb9dea17 --- /dev/null +++ b/tests/routing/internal-flag/chainsaw-test.yaml @@ -0,0 +1,18 @@ +apiVersion: chainsaw.kyverno.io/v1alpha1 +kind: Test +metadata: + name: routes +spec: + skip: false + concurrent: true + skipDelete: false + namespace: chainsaw-routing-internal-flag + steps: + - try: + - apply: + file: application.yaml + - apply: + file: routing-internal-flag.yaml + + - assert: + file: routing-internal-flag-assert.yaml diff --git a/tests/routing/internal-flag/routing-internal-flag-assert.yaml b/tests/routing/internal-flag/routing-internal-flag-assert.yaml new file mode 100644 index 00000000..b950e200 --- /dev/null +++ b/tests/routing/internal-flag/routing-internal-flag-assert.yaml @@ -0,0 +1,94 @@ +apiVersion: networking.istio.io/v1beta1 +kind: Gateway +metadata: + name: app-paths-routing-ingress +spec: + selector: + app: istio-ingress-internal + servers: + - hosts: + - example.com + port: + name: http + number: 80 + protocol: HTTP + - hosts: + - example.com + port: + name: https + number: 443 + protocol: HTTPS + tls: + credentialName: chainsaw-routing-internal-flag-app-paths-routing-ingre-9441fe86 + mode: SIMPLE + +--- +apiVersion: networking.istio.io/v1beta1 +kind: VirtualService +metadata: + name: app-paths-routing-ingress +spec: + exportTo: + - . + - istio-system + - istio-gateways + gateways: + - app-paths-routing-ingress + hosts: + - example.com + http: + - match: + - port: 80 + withoutHeaders: + :path: + prefix: /.well-known/acme-challenge/ + name: redirect-to-https + redirect: + redirectCode: 308 + scheme: https + - match: + - port: 443 + uri: + prefix: /app1 + rewrite: + uri: / + name: app-1 + route: + - destination: + host: app-1 + port: + number: 8000 + - match: + - port: 443 + uri: + prefix: /new-path + rewrite: + uri: / + name: app-2 + route: + - destination: + host: app-2 + port: + number: 9000 +--- +apiVersion: networking.k8s.io/v1 +kind: NetworkPolicy +metadata: + name: app-paths-app-2-istio-ingress +spec: + podSelector: + matchLabels: + app: app-2 + policyTypes: + - Ingress + ingress: + - from: + - namespaceSelector: + matchLabels: + kubernetes.io/metadata.name: istio-gateways + podSelector: + matchLabels: + app: istio-ingress-internal + ports: + - port: 9000 + protocol: TCP \ No newline at end of file diff --git a/tests/routing/internal-flag/routing-internal-flag.yaml b/tests/routing/internal-flag/routing-internal-flag.yaml new file mode 100644 index 00000000..a1d123c3 --- /dev/null +++ b/tests/routing/internal-flag/routing-internal-flag.yaml @@ -0,0 +1,15 @@ +apiVersion: skiperator.kartverket.no/v1alpha1 +kind: Routing +metadata: + name: app-paths +spec: + hostname: example.com + internal: true + routes: + - pathPrefix: /app1 + targetApp: app-1 + rewriteUri: true + - pathPrefix: /new-path + targetApp: app-2 + rewriteUri: true + From dc2d01d71c90084d09fdc5749af959d2d293a9ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A5rd=20Ove=20Hoel?= Date: Thu, 5 Sep 2024 09:19:31 +0200 Subject: [PATCH 05/34] added internal flag to routing --- api/v1alpha1/hosts.go | 26 +------------------ api/v1alpha1/routing_types.go | 3 +++ api/v1alpha1/zz_generated.deepcopy.go | 5 ++++ .../skiperator.kartverket.no_routings.yaml | 2 ++ .../istio/gateway/routing.go | 3 +++ .../networkpolicy/dynamic/routing.go | 3 +++ 6 files changed, 17 insertions(+), 25 deletions(-) diff --git a/api/v1alpha1/hosts.go b/api/v1alpha1/hosts.go index cd56bd98..9d21d1e8 100644 --- a/api/v1alpha1/hosts.go +++ b/api/v1alpha1/hosts.go @@ -30,8 +30,6 @@ func NewHost(hostname string) (*Host, error) { var h Host - h.Internal = IsInternal(hostname) - // If hostname is separated by +, the user wants to use a custom certificate results := strings.Split(hostname, hostnameSecretSeparator) @@ -56,7 +54,7 @@ func NewHost(hostname string) (*Host, error) { if err := domain.Check(h.Hostname); err != nil { return nil, fmt.Errorf("%s: failed validation: %w", h.Hostname, err) } - + h.Internal = IsInternal(hostname) return &h, nil } @@ -70,28 +68,6 @@ func NewCollection() HostCollection { } } -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) AddObject(hostname string, internal bool) error { h, err := NewHost(hostname) if err != nil { diff --git a/api/v1alpha1/routing_types.go b/api/v1alpha1/routing_types.go index cdb03de8..0ef283f5 100644 --- a/api/v1alpha1/routing_types.go +++ b/api/v1alpha1/routing_types.go @@ -41,6 +41,9 @@ type RoutingSpec struct { //+kubebuilder:validation:Optional //+kubebuilder:default:=true RedirectToHTTPS *bool `json:"redirectToHTTPS,omitempty"` + + //+kubebuilder:validation:Optional + Internal *bool `json:"internal,omitempty"` } // +kubebuilder:object:generate=true diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 7f454bef..824cdd42 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -495,6 +495,11 @@ func (in *RoutingSpec) DeepCopyInto(out *RoutingSpec) { *out = new(bool) **out = **in } + if in.Internal != nil { + in, out := &in.Internal, &out.Internal + *out = new(bool) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RoutingSpec. diff --git a/config/crd/skiperator.kartverket.no_routings.yaml b/config/crd/skiperator.kartverket.no_routings.yaml index abd2b035..a6608408 100644 --- a/config/crd/skiperator.kartverket.no_routings.yaml +++ b/config/crd/skiperator.kartverket.no_routings.yaml @@ -45,6 +45,8 @@ spec: properties: hostname: type: string + internal: + type: boolean redirectToHTTPS: default: true type: boolean diff --git a/pkg/resourcegenerator/istio/gateway/routing.go b/pkg/resourcegenerator/istio/gateway/routing.go index 3f08edba..920d6bb2 100644 --- a/pkg/resourcegenerator/istio/gateway/routing.go +++ b/pkg/resourcegenerator/istio/gateway/routing.go @@ -28,6 +28,9 @@ func generateForRouting(r reconciliation.Reconciliation) error { } h, err := routing.Spec.GetHost() + if routing.Spec.Internal != nil { + h.Internal = *routing.Spec.Internal + } if err != nil { return err } diff --git a/pkg/resourcegenerator/networkpolicy/dynamic/routing.go b/pkg/resourcegenerator/networkpolicy/dynamic/routing.go index 1a525731..8650ee09 100644 --- a/pkg/resourcegenerator/networkpolicy/dynamic/routing.go +++ b/pkg/resourcegenerator/networkpolicy/dynamic/routing.go @@ -28,6 +28,9 @@ func generateForRouting(r reconciliation.Reconciliation) error { uniqueTargetApps[getNetworkPolicyName(routing, route.TargetApp)] = route } h, err := routing.Spec.GetHost() + if routing.Spec.Internal != nil { + h.Internal = *routing.Spec.Internal + } if err != nil { return err } From 317898c71aaa4d9b9422a207bf5883b44eb1f0e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A5rd=20Ove=20Hoel?= Date: Tue, 10 Sep 2024 10:28:44 +0200 Subject: [PATCH 06/34] newlines --- tests/routing/internal-flag/routing-internal-flag-assert.yaml | 2 +- tests/routing/internal-flag/routing-internal-flag.yaml | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/routing/internal-flag/routing-internal-flag-assert.yaml b/tests/routing/internal-flag/routing-internal-flag-assert.yaml index b950e200..c5018b9b 100644 --- a/tests/routing/internal-flag/routing-internal-flag-assert.yaml +++ b/tests/routing/internal-flag/routing-internal-flag-assert.yaml @@ -91,4 +91,4 @@ spec: app: istio-ingress-internal ports: - port: 9000 - protocol: TCP \ No newline at end of file + protocol: TCP diff --git a/tests/routing/internal-flag/routing-internal-flag.yaml b/tests/routing/internal-flag/routing-internal-flag.yaml index a1d123c3..6d75510d 100644 --- a/tests/routing/internal-flag/routing-internal-flag.yaml +++ b/tests/routing/internal-flag/routing-internal-flag.yaml @@ -12,4 +12,3 @@ spec: - pathPrefix: /new-path targetApp: app-2 rewriteUri: true - From b55bad2f3feadbf5e628b0a223fcb2f4946a1135 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A5rd=20Ove=20Hoel?= Date: Tue, 10 Sep 2024 10:30:25 +0200 Subject: [PATCH 07/34] moved helperfunction --- pkg/resourcegenerator/istio/gateway/application.go | 3 ++- pkg/resourcegenerator/istio/gateway/routing.go | 2 +- pkg/resourcegenerator/networkpolicy/dynamic/routing.go | 3 ++- pkg/resourcegenerator/resourceutils/helpers.go | 8 ++++++++ pkg/util/helperfunctions.go | 8 -------- 5 files changed, 13 insertions(+), 11 deletions(-) diff --git a/pkg/resourcegenerator/istio/gateway/application.go b/pkg/resourcegenerator/istio/gateway/application.go index 39791107..c0e9aba6 100644 --- a/pkg/resourcegenerator/istio/gateway/application.go +++ b/pkg/resourcegenerator/istio/gateway/application.go @@ -4,6 +4,7 @@ import ( "fmt" skiperatorv1alpha1 "github.com/kartverket/skiperator/api/v1alpha1" "github.com/kartverket/skiperator/pkg/reconciliation" + "github.com/kartverket/skiperator/pkg/resourcegenerator/resourceutils" "github.com/kartverket/skiperator/pkg/util" networkingv1beta1api "istio.io/api/networking/v1beta1" networkingv1beta1 "istio.io/client-go/pkg/apis/networking/v1beta1" @@ -37,7 +38,7 @@ func generateForApplication(r reconciliation.Reconciliation) error { name := fmt.Sprintf("%s-ingress-%x", application.Name, util.GenerateHashFromName(h.Hostname)) gateway := networkingv1beta1.Gateway{ObjectMeta: metav1.ObjectMeta{Namespace: application.Namespace, Name: name}} - gateway.Spec.Selector = util.GetIstioGatewayLabelSelector(h) + gateway.Spec.Selector = resourceutils.GetIstioGatewayLabelSelector(h) gatewayServersToAdd := []*networkingv1beta1api.Server{} diff --git a/pkg/resourcegenerator/istio/gateway/routing.go b/pkg/resourcegenerator/istio/gateway/routing.go index 920d6bb2..e2382023 100644 --- a/pkg/resourcegenerator/istio/gateway/routing.go +++ b/pkg/resourcegenerator/istio/gateway/routing.go @@ -47,7 +47,7 @@ func generateForRouting(r reconciliation.Reconciliation) error { } } - gateway.Spec.Selector = util.GetIstioGatewayLabelSelector(h) + gateway.Spec.Selector = resourceutils.GetIstioGatewayLabelSelector(h) gateway.Spec.Servers = []*networkingv1beta1api.Server{ { Hosts: []string{h.Hostname}, diff --git a/pkg/resourcegenerator/networkpolicy/dynamic/routing.go b/pkg/resourcegenerator/networkpolicy/dynamic/routing.go index 8650ee09..64fd619d 100644 --- a/pkg/resourcegenerator/networkpolicy/dynamic/routing.go +++ b/pkg/resourcegenerator/networkpolicy/dynamic/routing.go @@ -5,6 +5,7 @@ import ( skiperatorv1alpha1 "github.com/kartverket/skiperator/api/v1alpha1" "github.com/kartverket/skiperator/pkg/reconciliation" + "github.com/kartverket/skiperator/pkg/resourcegenerator/resourceutils" "github.com/kartverket/skiperator/pkg/util" networkingv1 "k8s.io/api/networking/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -56,7 +57,7 @@ func generateForRouting(r reconciliation.Reconciliation) error { MatchLabels: util.GetIstioGatewaySelector(), }, PodSelector: &metav1.LabelSelector{ - MatchLabels: util.GetIstioGatewayLabelSelector(h), + MatchLabels: resourceutils.GetIstioGatewayLabelSelector(h), }, }, }, diff --git a/pkg/resourcegenerator/resourceutils/helpers.go b/pkg/resourcegenerator/resourceutils/helpers.go index d7bd46cf..8bfcb4a9 100644 --- a/pkg/resourcegenerator/resourceutils/helpers.go +++ b/pkg/resourcegenerator/resourceutils/helpers.go @@ -1,6 +1,7 @@ package resourceutils import ( + "github.com/kartverket/skiperator/api/v1alpha1" skiperatorv1alpha1 "github.com/kartverket/skiperator/api/v1alpha1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" ) @@ -16,3 +17,10 @@ func ShouldScaleToZero(jsonReplicas *apiextensionsv1.JSON) bool { } return false } + +func GetIstioGatewayLabelSelector(host *v1alpha1.Host) map[string]string { + if host.Internal { + return map[string]string{"app": "istio-ingress-internal"} + } + return map[string]string{"app": "istio-ingress-external"} +} diff --git a/pkg/util/helperfunctions.go b/pkg/util/helperfunctions.go index d0939183..9bd2ca4c 100644 --- a/pkg/util/helperfunctions.go +++ b/pkg/util/helperfunctions.go @@ -3,7 +3,6 @@ package util import ( "context" "fmt" - "github.com/kartverket/skiperator/api/v1alpha1" "github.com/kartverket/skiperator/api/v1alpha1/podtypes" "github.com/mitchellh/hashstructure/v2" "github.com/nais/liberator/pkg/namegen" @@ -17,13 +16,6 @@ import ( // TODO Clean up this file, move functions to more appropriate files -func GetIstioGatewayLabelSelector(host *v1alpha1.Host) map[string]string { - if host.Internal { - return map[string]string{"app": "istio-ingress-internal"} - } - return map[string]string{"app": "istio-ingress-external"} -} - func GetHashForStructs(obj []interface{}) string { hash, err := hashstructure.Hash(obj, hashstructure.FormatV2, nil) if err != nil { From 06294d3d5fceefe200c5373dcb76fcda60e61369 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A5rd=20Ove=20Hoel?= Date: Tue, 10 Sep 2024 10:32:11 +0200 Subject: [PATCH 08/34] errorhandling --- pkg/resourcegenerator/deployment/deployment.go | 1 + pkg/resourcegenerator/istio/gateway/routing.go | 9 ++++----- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/resourcegenerator/deployment/deployment.go b/pkg/resourcegenerator/deployment/deployment.go index e3a2cc9c..f7470430 100644 --- a/pkg/resourcegenerator/deployment/deployment.go +++ b/pkg/resourcegenerator/deployment/deployment.go @@ -207,6 +207,7 @@ func Generate(r reconciliation.Reconciliation) error { hosts, err := application.Spec.Hosts() if err != nil { ctxLog.Error(err, "could not get hosts from application") + return err } if deployment.Annotations == nil { deployment.Annotations = make(map[string]string) diff --git a/pkg/resourcegenerator/istio/gateway/routing.go b/pkg/resourcegenerator/istio/gateway/routing.go index e2382023..7212ef00 100644 --- a/pkg/resourcegenerator/istio/gateway/routing.go +++ b/pkg/resourcegenerator/istio/gateway/routing.go @@ -2,10 +2,9 @@ package gateway import ( "fmt" - skiperatorv1alpha1 "github.com/kartverket/skiperator/api/v1alpha1" "github.com/kartverket/skiperator/pkg/reconciliation" - "github.com/kartverket/skiperator/pkg/util" + "github.com/kartverket/skiperator/pkg/resourcegenerator/resourceutils" networkingv1beta1api "istio.io/api/networking/v1beta1" networkingv1beta1 "istio.io/client-go/pkg/apis/networking/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -28,12 +27,12 @@ func generateForRouting(r reconciliation.Reconciliation) error { } h, err := routing.Spec.GetHost() - if routing.Spec.Internal != nil { - h.Internal = *routing.Spec.Internal - } if err != nil { return err } + if routing.Spec.Internal != nil { + h.Internal = *routing.Spec.Internal + } gateway := networkingv1beta1.Gateway{ObjectMeta: metav1.ObjectMeta{Namespace: routing.Namespace, Name: routing.GetGatewayName()}} From ce2639c65090ef9219100850f04cbbbb06cf3189 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A5rd=20Ove=20Hoel?= Date: Tue, 10 Sep 2024 10:33:09 +0200 Subject: [PATCH 09/34] rename variables --- api/v1alpha1/application_types.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/api/v1alpha1/application_types.go b/api/v1alpha1/application_types.go index 9c8fede7..5b2b893c 100644 --- a/api/v1alpha1/application_types.go +++ b/api/v1alpha1/application_types.go @@ -450,23 +450,23 @@ func MarshalledIngresses(Ingresses interface{}) *apiextensionsv1.JSON { } func (s *ApplicationSpec) Hosts() (HostCollection, error) { - var resultString []string - var resultObject []Ingresses + var ingressesAsString []string + var ingressesAsObject []Ingresses var errorsFound []error hosts := NewCollection() ingresses := MarshalledIngresses(s.Ingresses) - err := json.Unmarshal(ingresses.Raw, &resultString) + err := json.Unmarshal(ingresses.Raw, &ingressesAsString) if err != nil { - err := json.Unmarshal(ingresses.Raw, &resultObject) + err := json.Unmarshal(ingresses.Raw, &ingressesAsObject) if err != nil { errorsFound = append(errorsFound, err) return NewCollection(), errors.Join(errorsFound...) } - for _, ingress := range resultObject { - err := hosts.AddObject(ingress.Hostname, ingress.Internal) + for _, ingress := range ingressesAsObject { + err := hosts.Add(ingress.Hostname, ingress.Internal) if err != nil { errorsFound = append(errorsFound, err) return NewCollection(), errors.Join(errorsFound...) @@ -475,8 +475,8 @@ func (s *ApplicationSpec) Hosts() (HostCollection, error) { return hosts, nil } - for _, ingress := range resultString { - err := hosts.AddObject(ingress, IsInternal(ingress)) + for _, ingress := range ingressesAsString { + err := hosts.Add(ingress, IsInternal(ingress)) if err != nil { errorsFound = append(errorsFound, err) } From 42aa262cc396259352b3e74bc446b287547ad9d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A5rd=20Ove=20Hoel?= Date: Tue, 10 Sep 2024 10:34:22 +0200 Subject: [PATCH 10/34] moved type def and corrected typo --- api/v1alpha1/application_types.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/api/v1alpha1/application_types.go b/api/v1alpha1/application_types.go index 5b2b893c..8ab7e2f5 100644 --- a/api/v1alpha1/application_types.go +++ b/api/v1alpha1/application_types.go @@ -326,6 +326,11 @@ type PrometheusConfig struct { AllowAllMetrics bool `json:"allowAllMetrics,omitempty"` } +type Ingresses struct { + Hostname string + Internal bool +} + func NewDefaultReplicas() Replicas { return Replicas{ Min: 2, @@ -432,16 +437,11 @@ func (a *Application) GetCommonSpec() *CommonSpec { } } -type Ingresses struct { - Hostname string - Internal bool -} - -func MarshalledIngresses(Ingresses interface{}) *apiextensionsv1.JSON { +func MarshalledIngresses(ingresses interface{}) *apiextensionsv1.JSON { IngressesJSON := &apiextensionsv1.JSON{} var err error - IngressesJSON.Raw, err = json.Marshal(Ingresses) + IngressesJSON.Raw, err = json.Marshal(ingresses) if err == nil { return IngressesJSON } From 49d30c134424c51f92e244195fe0d48f1c666632 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A5rd=20Ove=20Hoel?= Date: Tue, 10 Sep 2024 10:34:53 +0200 Subject: [PATCH 11/34] renamed function --- api/v1alpha1/hosts.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/v1alpha1/hosts.go b/api/v1alpha1/hosts.go index 9d21d1e8..eea2c827 100644 --- a/api/v1alpha1/hosts.go +++ b/api/v1alpha1/hosts.go @@ -68,7 +68,7 @@ func NewCollection() HostCollection { } } -func (hs *HostCollection) AddObject(hostname string, internal bool) error { +func (hs *HostCollection) Add(hostname string, internal bool) error { h, err := NewHost(hostname) if err != nil { return err From 16290f825da1c9c2dc90ec1c666cf25bea7e1057 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A5rd=20Ove=20Hoel?= Date: Mon, 16 Sep 2024 10:39:24 +0200 Subject: [PATCH 12/34] tests --- .../application-internal-flag-assert.yaml | 76 +++++++++++++++++++ .../application-internal-flag.yaml | 10 +++ ...internal-missing-internal-flag-assert.yaml | 76 +++++++++++++++++++ ...rnal-missing-internal-flag-ext-assert.yaml | 76 +++++++++++++++++++ ...on-internal-missing-internal-flag-ext.yaml | 9 +++ ...cation-internal-missing-internal-flag.yaml | 9 +++ .../internal-flag/chainsaw-test.yaml | 25 ++++++ .../routing/internal-flag/chainsaw-test.yaml | 3 +- 8 files changed, 282 insertions(+), 2 deletions(-) create mode 100644 tests/application/internal-flag/application-internal-flag-assert.yaml create mode 100644 tests/application/internal-flag/application-internal-flag.yaml create mode 100644 tests/application/internal-flag/application-internal-missing-internal-flag-assert.yaml create mode 100644 tests/application/internal-flag/application-internal-missing-internal-flag-ext-assert.yaml create mode 100644 tests/application/internal-flag/application-internal-missing-internal-flag-ext.yaml create mode 100644 tests/application/internal-flag/application-internal-missing-internal-flag.yaml create mode 100644 tests/application/internal-flag/chainsaw-test.yaml diff --git a/tests/application/internal-flag/application-internal-flag-assert.yaml b/tests/application/internal-flag/application-internal-flag-assert.yaml new file mode 100644 index 00000000..a1040c22 --- /dev/null +++ b/tests/application/internal-flag/application-internal-flag-assert.yaml @@ -0,0 +1,76 @@ +apiVersion: networking.istio.io/v1beta1 +kind: Gateway +metadata: + name: app-internal-flag-ingress-56cd7aa901014e78 +spec: + selector: + app: istio-ingress-internal + servers: + - hosts: + - example.com + port: + name: http + number: 80 + protocol: HTTP + - hosts: + - example.com + port: + name: https + number: 443 + protocol: HTTPS + tls: + credentialName: chainsaw-routing-internal-flag-app-internal-flag-ingress-56cd7aa901014e78 + mode: SIMPLE + +--- +apiVersion: networking.istio.io/v1beta1 +kind: VirtualService +metadata: + name: app-internal-flag-ingress +spec: + exportTo: + - . + - istio-system + - istio-gateways + gateways: + - app-internal-flag-ingress-56cd7aa901014e78 + hosts: + - example.com + 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: app-internal-flag + port: + number: 8000 +--- +apiVersion: networking.k8s.io/v1 +kind: NetworkPolicy +metadata: + name: app-internal-flag +spec: + ingress: + - from: + - namespaceSelector: + matchLabels: + kubernetes.io/metadata.name: istio-gateways + podSelector: + matchLabels: + app: istio-ingress-internal + ports: + - port: 8000 + protocol: TCP + podSelector: + matchLabels: + app: app-internal-flag + policyTypes: + - Ingress diff --git a/tests/application/internal-flag/application-internal-flag.yaml b/tests/application/internal-flag/application-internal-flag.yaml new file mode 100644 index 00000000..7c64d38f --- /dev/null +++ b/tests/application/internal-flag/application-internal-flag.yaml @@ -0,0 +1,10 @@ +apiVersion: skiperator.kartverket.no/v1alpha1 +kind: Application +metadata: + name: app-internal-flag +spec: + image: image + port: 8000 + ingresses: + - hostname: example.com + internal: true diff --git a/tests/application/internal-flag/application-internal-missing-internal-flag-assert.yaml b/tests/application/internal-flag/application-internal-missing-internal-flag-assert.yaml new file mode 100644 index 00000000..f64d59f0 --- /dev/null +++ b/tests/application/internal-flag/application-internal-missing-internal-flag-assert.yaml @@ -0,0 +1,76 @@ +apiVersion: networking.istio.io/v1beta1 +kind: Gateway +metadata: + name: app-missing-internal-flag-ingress-df26180a7fa049ff +spec: + selector: + app: istio-ingress-internal + servers: + - hosts: + - appmissinginternalflag.atkv3-dev.kartverket-intern.cloud + port: + name: http + number: 80 + protocol: HTTP + - hosts: + - appmissinginternalflag.atkv3-dev.kartverket-intern.cloud + port: + name: https + number: 443 + protocol: HTTPS + tls: + credentialName: chainsaw-routing-internal-flag-app-missing-internal-flag-ingress-df26180a7fa049ff + mode: SIMPLE + +--- +apiVersion: networking.istio.io/v1beta1 +kind: VirtualService +metadata: + name: app-missing-internal-flag-ingress +spec: + exportTo: + - . + - istio-system + - istio-gateways + gateways: + - app-missing-internal-flag-ingress-df26180a7fa049ff + hosts: + - appmissinginternalflag.atkv3-dev.kartverket-intern.cloud + 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: app-missing-internal-flag + port: + number: 8000 +--- +apiVersion: networking.k8s.io/v1 +kind: NetworkPolicy +metadata: + name: app-missing-internal-flag +spec: + ingress: + - from: + - namespaceSelector: + matchLabels: + kubernetes.io/metadata.name: istio-gateways + podSelector: + matchLabels: + app: istio-ingress-internal + ports: + - port: 8000 + protocol: TCP + podSelector: + matchLabels: + app: app-missing-internal-flag + policyTypes: + - Ingress diff --git a/tests/application/internal-flag/application-internal-missing-internal-flag-ext-assert.yaml b/tests/application/internal-flag/application-internal-missing-internal-flag-ext-assert.yaml new file mode 100644 index 00000000..2ce8a2e6 --- /dev/null +++ b/tests/application/internal-flag/application-internal-missing-internal-flag-ext-assert.yaml @@ -0,0 +1,76 @@ +apiVersion: networking.istio.io/v1beta1 +kind: Gateway +metadata: + name: app-missing-internal-flag-ingress-56cd7aa901014e78 +spec: + selector: + app: istio-ingress-external + servers: + - hosts: + - example.com + port: + name: http + number: 80 + protocol: HTTP + - hosts: + - example.com + port: + name: https + number: 443 + protocol: HTTPS + tls: + credentialName: chainsaw-routing-internal-flag-app-missing-internal-flag-ingress-56cd7aa901014e78 + mode: SIMPLE + +--- +apiVersion: networking.istio.io/v1beta1 +kind: VirtualService +metadata: + name: app-missing-internal-flag-ingress +spec: + exportTo: + - . + - istio-system + - istio-gateways + gateways: + - app-missing-internal-flag-ingress-56cd7aa901014e78 + hosts: + - example.com + 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: app-missing-internal-flag + port: + number: 8000 +--- +apiVersion: networking.k8s.io/v1 +kind: NetworkPolicy +metadata: + name: app-missing-internal-flag +spec: + ingress: + - from: + - namespaceSelector: + matchLabels: + kubernetes.io/metadata.name: istio-gateways + podSelector: + matchLabels: + app: istio-ingress-external + ports: + - port: 8000 + protocol: TCP + podSelector: + matchLabels: + app: app-missing-internal-flag + policyTypes: + - Ingress diff --git a/tests/application/internal-flag/application-internal-missing-internal-flag-ext.yaml b/tests/application/internal-flag/application-internal-missing-internal-flag-ext.yaml new file mode 100644 index 00000000..05d0305e --- /dev/null +++ b/tests/application/internal-flag/application-internal-missing-internal-flag-ext.yaml @@ -0,0 +1,9 @@ +apiVersion: skiperator.kartverket.no/v1alpha1 +kind: Application +metadata: + name: app-missing-internal-flag +spec: + image: image + port: 8000 + ingresses: + - hostname: example.com diff --git a/tests/application/internal-flag/application-internal-missing-internal-flag.yaml b/tests/application/internal-flag/application-internal-missing-internal-flag.yaml new file mode 100644 index 00000000..9c795d31 --- /dev/null +++ b/tests/application/internal-flag/application-internal-missing-internal-flag.yaml @@ -0,0 +1,9 @@ +apiVersion: skiperator.kartverket.no/v1alpha1 +kind: Application +metadata: + name: app-missing-internal-flag +spec: + image: image + port: 8000 + ingresses: + - hostname: appmissinginternalflag.atkv3-dev.kartverket-intern.cloud diff --git a/tests/application/internal-flag/chainsaw-test.yaml b/tests/application/internal-flag/chainsaw-test.yaml new file mode 100644 index 00000000..cfcb18e4 --- /dev/null +++ b/tests/application/internal-flag/chainsaw-test.yaml @@ -0,0 +1,25 @@ +apiVersion: chainsaw.kyverno.io/v1alpha1 +kind: Test +metadata: + name: routes +spec: + skip: false + concurrent: true + skipDelete: false + namespace: chainsaw-routing-internal-flag + steps: + - try: + - apply: + file: application-internal-flag.yaml + - assert: + file: application-internal-flag-assert.yaml + - try: + - apply: + file: application-internal-missing-internal-flag.yaml + - assert: + file: application-internal-missing-internal-flag-assert.yaml + - try: + - patch: + file: application-internal-missing-internal-flag-ext.yaml + - assert: + file: application-internal-missing-internal-flag-ext-assert.yaml diff --git a/tests/routing/internal-flag/chainsaw-test.yaml b/tests/routing/internal-flag/chainsaw-test.yaml index fb9dea17..5bb01f61 100644 --- a/tests/routing/internal-flag/chainsaw-test.yaml +++ b/tests/routing/internal-flag/chainsaw-test.yaml @@ -13,6 +13,5 @@ spec: file: application.yaml - apply: file: routing-internal-flag.yaml - - assert: - file: routing-internal-flag-assert.yaml + file: routing-internal-flag-assert.yaml \ No newline at end of file From 871f70c4edf56a44202c2cafbea1511943ef52fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A5rd=20Ove=20Hoel?= Date: Mon, 16 Sep 2024 10:40:40 +0200 Subject: [PATCH 13/34] pointer bool to enable nil check. --- api/v1alpha1/application_types.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/api/v1alpha1/application_types.go b/api/v1alpha1/application_types.go index 8ab7e2f5..61ae7eeb 100644 --- a/api/v1alpha1/application_types.go +++ b/api/v1alpha1/application_types.go @@ -5,6 +5,7 @@ import ( "errors" "github.com/kartverket/skiperator/api/v1alpha1/digdirator" "github.com/kartverket/skiperator/api/v1alpha1/podtypes" + "github.com/kartverket/skiperator/pkg/util" corev1 "k8s.io/api/core/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -328,7 +329,7 @@ type PrometheusConfig struct { type Ingresses struct { Hostname string - Internal bool + Internal *bool } func NewDefaultReplicas() Replicas { @@ -466,7 +467,11 @@ func (s *ApplicationSpec) Hosts() (HostCollection, error) { } for _, ingress := range ingressesAsObject { - err := hosts.Add(ingress.Hostname, ingress.Internal) + if ingress.Internal == nil { + ingress.Internal = util.PointTo(IsInternal(ingress.Hostname)) + } + + err := hosts.Add(ingress.Hostname, *ingress.Internal) if err != nil { errorsFound = append(errorsFound, err) return NewCollection(), errors.Join(errorsFound...) From 33ac843ec78c35a06fa3a2a61f5d5958f4770fd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A5rd=20Ove=20Hoel?= Date: Mon, 19 Aug 2024 10:58:30 +0200 Subject: [PATCH 14/34] Support for multiple types in ingresses --- api/v1alpha1/application_types.go | 57 ++++++++++++++++--- api/v1alpha1/zz_generated.deepcopy.go | 4 +- .../deployment/deployment.go | 10 +++- .../networkpolicy/dynamic/common.go | 10 +++- 4 files changed, 70 insertions(+), 11 deletions(-) diff --git a/api/v1alpha1/application_types.go b/api/v1alpha1/application_types.go index 47faa0e7..41022d67 100644 --- a/api/v1alpha1/application_types.go +++ b/api/v1alpha1/application_types.go @@ -3,6 +3,8 @@ package v1alpha1 import ( "encoding/json" "errors" + "fmt" + "time" "github.com/kartverket/skiperator/api/v1alpha1/digdirator" "github.com/kartverket/skiperator/api/v1alpha1/istiotypes" "github.com/kartverket/skiperator/api/v1alpha1/podtypes" @@ -10,7 +12,6 @@ import ( apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" - "time" ) // +kubebuilder:object:root=true @@ -68,7 +69,7 @@ type ApplicationSpec struct { // They can optionally be suffixed with a plus and name of a custom TLS secret located in the istio-gateways namespace. // E.g. "foo.atkv3-dev.kartverket-intern.cloud+env-wildcard-cert" //+kubebuilder:validation:Optional - Ingresses []string `json:"ingresses,omitempty"` + Ingresses *apiextensionsv1.JSON `json:"ingresses,omitempty"` // An optional priority. Supported values are 'low', 'medium' and 'high'. // The default value is 'medium'. @@ -442,19 +443,61 @@ func (a *Application) GetCommonSpec() *CommonSpec { } } +type Ingresses struct { + Hostname string + Internal bool +} + +func MarshalledIngresses(Ingresses interface{}) *apiextensionsv1.JSON { + IngressesJSON := &apiextensionsv1.JSON{} + var err error + + IngressesJSON.Raw, err = json.Marshal(Ingresses) + if err == nil { + return IngressesJSON + } + + return nil +} + func (s *ApplicationSpec) Hosts() (HostCollection, error) { + var resultString []string + var resultObject []Ingresses + var errorsFound []error + hosts := NewCollection() - var errorsFound []error - for _, ingress := range s.Ingresses { + ingresses := MarshalledIngresses(s.Ingresses) + err := json.Unmarshal(ingresses.Raw, &resultString) + + + if err != nil { + err := json.Unmarshal(ingresses.Raw, &resultObject) + if err != nil { + errorsFound = append(errorsFound, err) + return HostCollection{}, errors.Join(errorsFound...) + } + + + for _, ingress := range resultObject { + fmt.Printf("############################### %v\n", ingress.Hostname) + err := hosts.Add(ingress.Hostname) + if err != nil { + errorsFound = append(errorsFound, err) + } + } + return HostCollection{}, nil + } + + for _, ingress := range resultString { + fmt.Printf("############################### %v\n", ingress) err := hosts.Add(ingress) if err != nil { errorsFound = append(errorsFound, err) - continue } } - - return hosts, errors.Join(errorsFound...) + + return HostCollection{}, nil } type MultiErr interface { diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index a54de73f..b9b64502 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -78,8 +78,8 @@ func (in *ApplicationSpec) DeepCopyInto(out *ApplicationSpec) { *out = *in if in.Ingresses != nil { in, out := &in.Ingresses, &out.Ingresses - *out = make([]string, len(*in)) - copy(*out, *in) + *out = new(v1.JSON) + (*in).DeepCopyInto(*out) } if in.Command != nil { in, out := &in.Command, &out.Command diff --git a/pkg/resourcegenerator/deployment/deployment.go b/pkg/resourcegenerator/deployment/deployment.go index bb0d6199..bc0b8e45 100644 --- a/pkg/resourcegenerator/deployment/deployment.go +++ b/pkg/resourcegenerator/deployment/deployment.go @@ -204,10 +204,18 @@ func Generate(r reconciliation.Reconciliation) error { } // add an external link to argocd - ingresses := application.Spec.Ingresses + hosts, err := application.Spec.Hosts() + if err != nil { + ctxLog.Error(err, "could not get hosts from application") + } if deployment.Annotations == nil { deployment.Annotations = make(map[string]string) } + + var ingresses []string + for _, host := range hosts.AllHosts() { + ingresses = append(ingresses, host.Hostname) + } if len(ingresses) > 0 { deployment.Annotations[AnnotationKeyLinkPrefix] = fmt.Sprintf("https://%s", ingresses[0]) diff --git a/pkg/resourcegenerator/networkpolicy/dynamic/common.go b/pkg/resourcegenerator/networkpolicy/dynamic/common.go index 313204a9..5f6d435d 100644 --- a/pkg/resourcegenerator/networkpolicy/dynamic/common.go +++ b/pkg/resourcegenerator/networkpolicy/dynamic/common.go @@ -39,8 +39,16 @@ func generateForCommon(r reconciliation.Reconciliation) error { accessPolicy := object.GetCommonSpec().AccessPolicy var ingresses []string var inboundPort int32 + + hosts, err := object.(*skiperatorv1alpha1.Application).Spec.Hosts() + if err != nil { + ctxLog.Error(err, "Failed to get hosts for networkpolicy", "type", r.GetType(), "namespace", namespace) + } + + for _, host := range hosts.AllHosts() { + ingresses = append(ingresses, host.Hostname) + } if r.GetType() == reconciliation.ApplicationType { - ingresses = object.(*skiperatorv1alpha1.Application).Spec.Ingresses inboundPort = int32(object.(*skiperatorv1alpha1.Application).Spec.Port) } From 9e2889ec3edd7e4b362fb9bdb624857bcbf9273b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A5rd=20Ove=20Hoel?= Date: Tue, 3 Sep 2024 08:53:10 +0200 Subject: [PATCH 15/34] changes --- api/v1alpha1/application_types.go | 27 ++++------ api/v1alpha1/hosts.go | 33 ++++++++++++ .../deployment/deployment.go | 7 +-- .../istio/gateway/application.go | 3 +- .../istio/gateway/routing.go | 2 +- .../networkpolicy/dynamic/common.go | 53 +++++-------------- .../networkpolicy/dynamic/routing.go | 7 ++- pkg/util/helperfunctions.go | 30 +++++++---- ...application-is-internal-vanity-assert.yaml | 46 ++++++++++++++++ .../application-is-internal-vanity.yaml | 10 ++++ tests/application/ingress/chainsaw-test.yaml | 5 ++ 11 files changed, 147 insertions(+), 76 deletions(-) create mode 100644 tests/application/ingress/application-is-internal-vanity-assert.yaml create mode 100644 tests/application/ingress/application-is-internal-vanity.yaml diff --git a/api/v1alpha1/application_types.go b/api/v1alpha1/application_types.go index 41022d67..3a33563d 100644 --- a/api/v1alpha1/application_types.go +++ b/api/v1alpha1/application_types.go @@ -3,8 +3,6 @@ package v1alpha1 import ( "encoding/json" "errors" - "fmt" - "time" "github.com/kartverket/skiperator/api/v1alpha1/digdirator" "github.com/kartverket/skiperator/api/v1alpha1/istiotypes" "github.com/kartverket/skiperator/api/v1alpha1/podtypes" @@ -12,6 +10,7 @@ import ( apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" + "time" ) // +kubebuilder:object:root=true @@ -464,40 +463,36 @@ func (s *ApplicationSpec) Hosts() (HostCollection, error) { var resultString []string var resultObject []Ingresses var errorsFound []error - hosts := NewCollection() ingresses := MarshalledIngresses(s.Ingresses) err := json.Unmarshal(ingresses.Raw, &resultString) - - + if err != nil { err := json.Unmarshal(ingresses.Raw, &resultObject) if err != nil { errorsFound = append(errorsFound, err) - return HostCollection{}, errors.Join(errorsFound...) + return NewCollection(), errors.Join(errorsFound...) } - - + for _, ingress := range resultObject { - fmt.Printf("############################### %v\n", ingress.Hostname) - err := hosts.Add(ingress.Hostname) + err := hosts.AddObject(ingress.Hostname, ingress.Internal) if err != nil { errorsFound = append(errorsFound, err) + return NewCollection(), errors.Join(errorsFound...) } } - return HostCollection{}, nil + return hosts, nil } - + for _, ingress := range resultString { - fmt.Printf("############################### %v\n", ingress) - err := hosts.Add(ingress) + err := hosts.AddObject(ingress, IsInternal(ingress)) if err != nil { errorsFound = append(errorsFound, err) } } - - return HostCollection{}, nil + + return hosts, errors.Join(errorsFound...) } type MultiErr interface { diff --git a/api/v1alpha1/hosts.go b/api/v1alpha1/hosts.go index 15cc531e..cd56bd98 100644 --- a/api/v1alpha1/hosts.go +++ b/api/v1alpha1/hosts.go @@ -3,16 +3,20 @@ package v1alpha1 import ( "fmt" "github.com/chmike/domain" + "regexp" "strings" ) const hostnameSecretSeparator = "+" +var internalPattern = regexp.MustCompile(`[^.]\.skip\.statkart\.no|[^.]\.kartverket-intern.cloud`) + // TODO: Add a mechanism for validating that the // hostname is covered by the CustomCertificateSecret if present type Host struct { Hostname string CustomCertificateSecret *string + Internal bool } type HostCollection struct { @@ -25,6 +29,9 @@ func NewHost(hostname string) (*Host, error) { } var h Host + + h.Internal = IsInternal(hostname) + // If hostname is separated by +, the user wants to use a custom certificate results := strings.Split(hostname, hostnameSecretSeparator) @@ -70,7 +77,29 @@ func (hs *HostCollection) Add(hostname string) error { } 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) AddObject(hostname string, internal bool) error { + h, err := NewHost(hostname) + if err != nil { + return err + } + h.Internal = internal + existingValue, alreadyPresent := hs.hosts[h.Hostname] switch alreadyPresent { case true: if existingValue.UsesCustomCert() { @@ -105,3 +134,7 @@ func (hs *HostCollection) Hostnames() []string { func (hs *HostCollection) Count() int { return len(hs.hosts) } + +func IsInternal(hostname string) bool { + return internalPattern.MatchString(hostname) +} diff --git a/pkg/resourcegenerator/deployment/deployment.go b/pkg/resourcegenerator/deployment/deployment.go index bc0b8e45..e3a2cc9c 100644 --- a/pkg/resourcegenerator/deployment/deployment.go +++ b/pkg/resourcegenerator/deployment/deployment.go @@ -211,11 +211,8 @@ func Generate(r reconciliation.Reconciliation) error { if deployment.Annotations == nil { deployment.Annotations = make(map[string]string) } - - var ingresses []string - for _, host := range hosts.AllHosts() { - ingresses = append(ingresses, host.Hostname) - } + + var ingresses = hosts.Hostnames() if len(ingresses) > 0 { deployment.Annotations[AnnotationKeyLinkPrefix] = fmt.Sprintf("https://%s", ingresses[0]) diff --git a/pkg/resourcegenerator/istio/gateway/application.go b/pkg/resourcegenerator/istio/gateway/application.go index 51460698..39791107 100644 --- a/pkg/resourcegenerator/istio/gateway/application.go +++ b/pkg/resourcegenerator/istio/gateway/application.go @@ -2,7 +2,6 @@ package gateway import ( "fmt" - skiperatorv1alpha1 "github.com/kartverket/skiperator/api/v1alpha1" "github.com/kartverket/skiperator/pkg/reconciliation" "github.com/kartverket/skiperator/pkg/util" @@ -38,7 +37,7 @@ func generateForApplication(r reconciliation.Reconciliation) error { name := fmt.Sprintf("%s-ingress-%x", application.Name, util.GenerateHashFromName(h.Hostname)) gateway := networkingv1beta1.Gateway{ObjectMeta: metav1.ObjectMeta{Namespace: application.Namespace, Name: name}} - gateway.Spec.Selector = util.GetIstioGatewayLabelSelector(h.Hostname) + gateway.Spec.Selector = util.GetIstioGatewayLabelSelector(h) gatewayServersToAdd := []*networkingv1beta1api.Server{} diff --git a/pkg/resourcegenerator/istio/gateway/routing.go b/pkg/resourcegenerator/istio/gateway/routing.go index 714dbc87..3f08edba 100644 --- a/pkg/resourcegenerator/istio/gateway/routing.go +++ b/pkg/resourcegenerator/istio/gateway/routing.go @@ -44,7 +44,7 @@ func generateForRouting(r reconciliation.Reconciliation) error { } } - gateway.Spec.Selector = util.GetIstioGatewayLabelSelector(h.Hostname) + gateway.Spec.Selector = util.GetIstioGatewayLabelSelector(h) gateway.Spec.Servers = []*networkingv1beta1api.Server{ { Hosts: []string{h.Hostname}, diff --git a/pkg/resourcegenerator/networkpolicy/dynamic/common.go b/pkg/resourcegenerator/networkpolicy/dynamic/common.go index 5f6d435d..998f265d 100644 --- a/pkg/resourcegenerator/networkpolicy/dynamic/common.go +++ b/pkg/resourcegenerator/networkpolicy/dynamic/common.go @@ -37,22 +37,20 @@ func generateForCommon(r reconciliation.Reconciliation) error { } accessPolicy := object.GetCommonSpec().AccessPolicy - var ingresses []string + var hosts = skiperatorv1alpha1.NewCollection() var inboundPort int32 - hosts, err := object.(*skiperatorv1alpha1.Application).Spec.Hosts() - if err != nil { - ctxLog.Error(err, "Failed to get hosts for networkpolicy", "type", r.GetType(), "namespace", namespace) - } - - for _, host := range hosts.AllHosts() { - ingresses = append(ingresses, host.Hostname) - } if r.GetType() == reconciliation.ApplicationType { + var err error + hosts, err = object.(*skiperatorv1alpha1.Application).Spec.Hosts() + if err != nil { + ctxLog.Error(err, "Failed to get hosts for networkpolicy", "type", r.GetType(), "namespace", namespace) + return err + } inboundPort = int32(object.(*skiperatorv1alpha1.Application).Spec.Port) } - ingressRules := getIngressRules(accessPolicy, ingresses, r.IsIstioEnabled(), namespace, inboundPort) + ingressRules := getIngressRules(accessPolicy, hosts, r.IsIstioEnabled(), namespace, inboundPort) egressRules := getEgressRules(accessPolicy, namespace) netpolSpec := networkingv1.NetworkPolicySpec{ @@ -121,20 +119,14 @@ func getEgressRule(outboundRule podtypes.InternalRule, namespace string) network } // TODO Clean up better -func getIngressRules(accessPolicy *podtypes.AccessPolicy, ingresses []string, istioEnabled bool, namespace string, port int32) []networkingv1.NetworkPolicyIngressRule { +func getIngressRules(accessPolicy *podtypes.AccessPolicy, hostCollection skiperatorv1alpha1.HostCollection, istioEnabled bool, namespace string, port int32) []networkingv1.NetworkPolicyIngressRule { var ingressRules []networkingv1.NetworkPolicyIngressRule - if ingresses != nil && len(ingresses) > 0 { - if hasInternalIngress(ingresses) { - ingressRules = append(ingressRules, getGatewayIngressRule(true, port)) - } - - if hasExternalIngress(ingresses) { - ingressRules = append(ingressRules, getGatewayIngressRule(false, port)) - } + for _, host := range hostCollection.AllHosts() { + ingressRules = append(ingressRules, getGatewayIngressRule(host.Internal, port)) } - // Allow grafana-agent to scrape + // Allow grafana-agent and Alloy to scrape if istioEnabled { promScrapeRuleGrafana := networkingv1.NetworkPolicyIngressRule{ From: []networkingv1.NetworkPolicyPeer{ @@ -234,26 +226,6 @@ func getNamespaceSelector(rule podtypes.InternalRule, appNamespace string) *meta } } -func hasExternalIngress(ingresses []string) bool { - for _, hostname := range ingresses { - if !util.IsInternal(hostname) { - return true - } - } - - return false -} - -func hasInternalIngress(ingresses []string) bool { - for _, hostname := range ingresses { - if util.IsInternal(hostname) { - return true - } - } - - return false -} - func getGatewayIngressRule(isInternal bool, port int32) networkingv1.NetworkPolicyIngressRule { ingressRule := networkingv1.NetworkPolicyIngressRule{ From: []networkingv1.NetworkPolicyPeer{ @@ -262,6 +234,7 @@ func getGatewayIngressRule(isInternal bool, port int32) networkingv1.NetworkPoli MatchLabels: map[string]string{"kubernetes.io/metadata.name": "istio-gateways"}, }, PodSelector: &metav1.LabelSelector{ + MatchLabels: getIngressGatewayLabel(isInternal), }, }, diff --git a/pkg/resourcegenerator/networkpolicy/dynamic/routing.go b/pkg/resourcegenerator/networkpolicy/dynamic/routing.go index 11fd658a..1a525731 100644 --- a/pkg/resourcegenerator/networkpolicy/dynamic/routing.go +++ b/pkg/resourcegenerator/networkpolicy/dynamic/routing.go @@ -27,7 +27,10 @@ func generateForRouting(r reconciliation.Reconciliation) error { for _, route := range routing.Spec.Routes { uniqueTargetApps[getNetworkPolicyName(routing, route.TargetApp)] = route } - + h, err := routing.Spec.GetHost() + if err != nil { + return err + } for netpolName, route := range uniqueTargetApps { networkPolicy := networkingv1.NetworkPolicy{ ObjectMeta: metav1.ObjectMeta{ @@ -50,7 +53,7 @@ func generateForRouting(r reconciliation.Reconciliation) error { MatchLabels: util.GetIstioGatewaySelector(), }, PodSelector: &metav1.LabelSelector{ - MatchLabels: util.GetIstioGatewayLabelSelector(routing.Spec.Hostname), + MatchLabels: util.GetIstioGatewayLabelSelector(h), }, }, }, diff --git a/pkg/util/helperfunctions.go b/pkg/util/helperfunctions.go index a3bf824e..b54e872a 100644 --- a/pkg/util/helperfunctions.go +++ b/pkg/util/helperfunctions.go @@ -3,6 +3,7 @@ package util import ( "context" "fmt" + "github.com/kartverket/skiperator/api/v1alpha1" "github.com/kartverket/skiperator/api/v1alpha1/podtypes" "github.com/mitchellh/hashstructure/v2" "github.com/nais/liberator/pkg/namegen" @@ -13,26 +14,20 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/validation" "k8s.io/client-go/tools/record" + "reflect" "regexp" "sigs.k8s.io/controller-runtime/pkg/client" "strings" "unicode" ) -//TODO Clean up this file, move functions to more appropriate files +// TODO Clean up this file, move functions to more appropriate files -var internalPattern = regexp.MustCompile(`[^.]\.skip\.statkart\.no|[^.]\.kartverket-intern.cloud`) - -func IsInternal(hostname string) bool { - return internalPattern.MatchString(hostname) -} - -func GetIstioGatewayLabelSelector(hostname string) map[string]string { - if IsInternal(hostname) { +func GetIstioGatewayLabelSelector(host *v1alpha1.Host) map[string]string { + if host.Internal { return map[string]string{"app": "istio-ingress-internal"} } return map[string]string{"app": "istio-ingress-external"} - } func GetHashForStructs(obj []interface{}) string { @@ -148,6 +143,21 @@ func EnsurePrefix(s string, prefix string) string { return s } +func GetObjectDiff[T any](a T, b T) (diff.Changelog, error) { + aKind := reflect.ValueOf(a).Kind() + bKind := reflect.ValueOf(b).Kind() + if aKind != bKind { + return nil, fmt.Errorf("The objects to compare are not the same, found obj1: %v, obj2: %v\n", aKind, bKind) + } + changelog, err := diff.Diff(a, b) + + if len(changelog) == 0 { + return nil, err + } + + return changelog, nil +} + func IsCloudSqlProxyEnabled(gcp *podtypes.GCP) bool { return gcp != nil && gcp.CloudSQLProxy != nil } diff --git a/tests/application/ingress/application-is-internal-vanity-assert.yaml b/tests/application/ingress/application-is-internal-vanity-assert.yaml new file mode 100644 index 00000000..53abe1af --- /dev/null +++ b/tests/application/ingress/application-is-internal-vanity-assert.yaml @@ -0,0 +1,46 @@ +apiVersion: networking.istio.io/v1beta1 +kind: Gateway +metadata: + name: internalvanity-ingress-dc2b250f77a411ad +spec: + selector: + app: istio-ingress-internal +--- +apiVersion: networking.k8s.io/v1 +kind: NetworkPolicy +metadata: + name: internalvanity +spec: + ingress: + - from: + - namespaceSelector: + matchLabels: + kubernetes.io/metadata.name: istio-gateways + podSelector: + matchLabels: + app: istio-ingress-internal + podSelector: + matchLabels: + app: internalvanity +--- +apiVersion: networking.k8s.io/v1 +kind: NetworkPolicy +metadata: + name: internalvanity +spec: + ingress: + - from: + - namespaceSelector: + matchLabels: + kubernetes.io/metadata.name: istio-gateways + podSelector: + matchLabels: + app: istio-ingress-internal + ports: + - port: 8080 + protocol: TCP + podSelector: + matchLabels: + app: internalvanity + policyTypes: + - Ingress diff --git a/tests/application/ingress/application-is-internal-vanity.yaml b/tests/application/ingress/application-is-internal-vanity.yaml new file mode 100644 index 00000000..fe3f59e5 --- /dev/null +++ b/tests/application/ingress/application-is-internal-vanity.yaml @@ -0,0 +1,10 @@ +apiVersion: skiperator.kartverket.no/v1alpha1 +kind: Application +metadata: + name: internalvanity +spec: + image: image + port: 8080 + ingresses: + - hostname: test.kartverket.no + internal: true \ No newline at end of file diff --git a/tests/application/ingress/chainsaw-test.yaml b/tests/application/ingress/chainsaw-test.yaml index d23d073a..7e911029 100644 --- a/tests/application/ingress/chainsaw-test.yaml +++ b/tests/application/ingress/chainsaw-test.yaml @@ -55,3 +55,8 @@ spec: file: application-is-external.yaml - assert: file: application-is-external-assert.yaml + - try: + - create: + file: application-is-internal-vanity.yaml + - assert: + file: application-is-internal-vanity-assert.yaml From ca300990990408c8c7fd68066d08a26fc18519c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A5rd=20Ove=20Hoel?= Date: Wed, 4 Sep 2024 10:06:58 +0200 Subject: [PATCH 16/34] Removed unused functions --- pkg/util/helperfunctions.go | 64 ------------------------------------- 1 file changed, 64 deletions(-) diff --git a/pkg/util/helperfunctions.go b/pkg/util/helperfunctions.go index b54e872a..d0939183 100644 --- a/pkg/util/helperfunctions.go +++ b/pkg/util/helperfunctions.go @@ -9,16 +9,10 @@ import ( "github.com/nais/liberator/pkg/namegen" "hash/fnv" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/validation" - "k8s.io/client-go/tools/record" - "reflect" - "regexp" "sigs.k8s.io/controller-runtime/pkg/client" "strings" - "unicode" ) // TODO Clean up this file, move functions to more appropriate files @@ -52,35 +46,6 @@ func GetConfigMap(client client.Client, ctx context.Context, namespacedName type return configMap, err } -func GetSecret(client client.Client, ctx context.Context, namespacedName types.NamespacedName) (corev1.Secret, error) { - secret := corev1.Secret{} - - err := client.Get(ctx, namespacedName, &secret) - - return secret, err -} - -func GetService(client client.Client, ctx context.Context, namespacedName types.NamespacedName) (corev1.Service, error) { - service := corev1.Service{} - - err := client.Get(ctx, namespacedName, &service) - - return service, err -} - -func ErrIsMissingOrNil(recorder record.EventRecorder, err error, message string, object runtime.Object) bool { - if errors.IsNotFound(err) { - recorder.Eventf( - object, - corev1.EventTypeWarning, "Missing", - message, - ) - } else if err != nil { - return false - } - return true -} - func ErrDoPanic(err error, message string) { if err != nil { errorMessage := fmt.Sprintf(message, err.Error()) @@ -111,24 +76,10 @@ func GetPodAppAndTeamSelector(applicationName string, teamName string) map[strin } } -func HasUpperCaseLetter(word string) bool { - for _, letter := range word { - if unicode.IsUpper(letter) { - return true - } - } - - return false -} - func ResourceNameWithKindPostfix(resourceName string, kind string) string { return strings.ToLower(fmt.Sprintf("%v-%v", resourceName, kind)) } -func GetGatewaySecretName(namespace string, name string) string { - return fmt.Sprintf("%s-%s-ingress", namespace, name) -} - func GetSecretName(prefix string, name string) (string, error) { // https://github.com/nais/naiserator/blob/faed273b68dff8541e1e2889fda5d017730f9796/pkg/resourcecreator/idporten/idporten.go#L82 // https://github.com/nais/naiserator/blob/faed273b68dff8541e1e2889fda5d017730f9796/pkg/resourcecreator/idporten/idporten.go#L170 @@ -143,21 +94,6 @@ func EnsurePrefix(s string, prefix string) string { return s } -func GetObjectDiff[T any](a T, b T) (diff.Changelog, error) { - aKind := reflect.ValueOf(a).Kind() - bKind := reflect.ValueOf(b).Kind() - if aKind != bKind { - return nil, fmt.Errorf("The objects to compare are not the same, found obj1: %v, obj2: %v\n", aKind, bKind) - } - changelog, err := diff.Diff(a, b) - - if len(changelog) == 0 { - return nil, err - } - - return changelog, nil -} - func IsCloudSqlProxyEnabled(gcp *podtypes.GCP) bool { return gcp != nil && gcp.CloudSQLProxy != nil } From 387fa007bbed722159da0cb703e1e03a1faa3807 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A5rd=20Ove=20Hoel?= Date: Thu, 5 Sep 2024 09:17:32 +0200 Subject: [PATCH 17/34] tests --- tests/routing/internal-flag/application.yaml | 15 +++ .../routing/internal-flag/chainsaw-test.yaml | 18 ++++ .../routing-internal-flag-assert.yaml | 94 +++++++++++++++++++ .../internal-flag/routing-internal-flag.yaml | 15 +++ 4 files changed, 142 insertions(+) create mode 100644 tests/routing/internal-flag/application.yaml create mode 100644 tests/routing/internal-flag/chainsaw-test.yaml create mode 100644 tests/routing/internal-flag/routing-internal-flag-assert.yaml create mode 100644 tests/routing/internal-flag/routing-internal-flag.yaml diff --git a/tests/routing/internal-flag/application.yaml b/tests/routing/internal-flag/application.yaml new file mode 100644 index 00000000..32c89a0f --- /dev/null +++ b/tests/routing/internal-flag/application.yaml @@ -0,0 +1,15 @@ +apiVersion: skiperator.kartverket.no/v1alpha1 +kind: Application +metadata: + name: app-1 +spec: + image: gcr.io/google-samples/istio/helloserver:v0.0.1 + port: 8000 +--- +apiVersion: skiperator.kartverket.no/v1alpha1 +kind: Application +metadata: + name: app-2 +spec: + image: gcr.io/google-samples/istio/helloserver:v0.0.1 + port: 9000 diff --git a/tests/routing/internal-flag/chainsaw-test.yaml b/tests/routing/internal-flag/chainsaw-test.yaml new file mode 100644 index 00000000..fb9dea17 --- /dev/null +++ b/tests/routing/internal-flag/chainsaw-test.yaml @@ -0,0 +1,18 @@ +apiVersion: chainsaw.kyverno.io/v1alpha1 +kind: Test +metadata: + name: routes +spec: + skip: false + concurrent: true + skipDelete: false + namespace: chainsaw-routing-internal-flag + steps: + - try: + - apply: + file: application.yaml + - apply: + file: routing-internal-flag.yaml + + - assert: + file: routing-internal-flag-assert.yaml diff --git a/tests/routing/internal-flag/routing-internal-flag-assert.yaml b/tests/routing/internal-flag/routing-internal-flag-assert.yaml new file mode 100644 index 00000000..b950e200 --- /dev/null +++ b/tests/routing/internal-flag/routing-internal-flag-assert.yaml @@ -0,0 +1,94 @@ +apiVersion: networking.istio.io/v1beta1 +kind: Gateway +metadata: + name: app-paths-routing-ingress +spec: + selector: + app: istio-ingress-internal + servers: + - hosts: + - example.com + port: + name: http + number: 80 + protocol: HTTP + - hosts: + - example.com + port: + name: https + number: 443 + protocol: HTTPS + tls: + credentialName: chainsaw-routing-internal-flag-app-paths-routing-ingre-9441fe86 + mode: SIMPLE + +--- +apiVersion: networking.istio.io/v1beta1 +kind: VirtualService +metadata: + name: app-paths-routing-ingress +spec: + exportTo: + - . + - istio-system + - istio-gateways + gateways: + - app-paths-routing-ingress + hosts: + - example.com + http: + - match: + - port: 80 + withoutHeaders: + :path: + prefix: /.well-known/acme-challenge/ + name: redirect-to-https + redirect: + redirectCode: 308 + scheme: https + - match: + - port: 443 + uri: + prefix: /app1 + rewrite: + uri: / + name: app-1 + route: + - destination: + host: app-1 + port: + number: 8000 + - match: + - port: 443 + uri: + prefix: /new-path + rewrite: + uri: / + name: app-2 + route: + - destination: + host: app-2 + port: + number: 9000 +--- +apiVersion: networking.k8s.io/v1 +kind: NetworkPolicy +metadata: + name: app-paths-app-2-istio-ingress +spec: + podSelector: + matchLabels: + app: app-2 + policyTypes: + - Ingress + ingress: + - from: + - namespaceSelector: + matchLabels: + kubernetes.io/metadata.name: istio-gateways + podSelector: + matchLabels: + app: istio-ingress-internal + ports: + - port: 9000 + protocol: TCP \ No newline at end of file diff --git a/tests/routing/internal-flag/routing-internal-flag.yaml b/tests/routing/internal-flag/routing-internal-flag.yaml new file mode 100644 index 00000000..a1d123c3 --- /dev/null +++ b/tests/routing/internal-flag/routing-internal-flag.yaml @@ -0,0 +1,15 @@ +apiVersion: skiperator.kartverket.no/v1alpha1 +kind: Routing +metadata: + name: app-paths +spec: + hostname: example.com + internal: true + routes: + - pathPrefix: /app1 + targetApp: app-1 + rewriteUri: true + - pathPrefix: /new-path + targetApp: app-2 + rewriteUri: true + From 0a031d0297b61c5e637ac7b511fd48e16b5fb4e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A5rd=20Ove=20Hoel?= Date: Thu, 5 Sep 2024 09:19:31 +0200 Subject: [PATCH 18/34] added internal flag to routing --- api/v1alpha1/hosts.go | 26 +------------------ api/v1alpha1/routing_types.go | 3 +++ api/v1alpha1/zz_generated.deepcopy.go | 5 ++++ .../skiperator.kartverket.no_routings.yaml | 2 ++ .../istio/gateway/routing.go | 3 +++ .../networkpolicy/dynamic/routing.go | 3 +++ 6 files changed, 17 insertions(+), 25 deletions(-) diff --git a/api/v1alpha1/hosts.go b/api/v1alpha1/hosts.go index cd56bd98..9d21d1e8 100644 --- a/api/v1alpha1/hosts.go +++ b/api/v1alpha1/hosts.go @@ -30,8 +30,6 @@ func NewHost(hostname string) (*Host, error) { var h Host - h.Internal = IsInternal(hostname) - // If hostname is separated by +, the user wants to use a custom certificate results := strings.Split(hostname, hostnameSecretSeparator) @@ -56,7 +54,7 @@ func NewHost(hostname string) (*Host, error) { if err := domain.Check(h.Hostname); err != nil { return nil, fmt.Errorf("%s: failed validation: %w", h.Hostname, err) } - + h.Internal = IsInternal(hostname) return &h, nil } @@ -70,28 +68,6 @@ func NewCollection() HostCollection { } } -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) AddObject(hostname string, internal bool) error { h, err := NewHost(hostname) if err != nil { diff --git a/api/v1alpha1/routing_types.go b/api/v1alpha1/routing_types.go index cdb03de8..0ef283f5 100644 --- a/api/v1alpha1/routing_types.go +++ b/api/v1alpha1/routing_types.go @@ -41,6 +41,9 @@ type RoutingSpec struct { //+kubebuilder:validation:Optional //+kubebuilder:default:=true RedirectToHTTPS *bool `json:"redirectToHTTPS,omitempty"` + + //+kubebuilder:validation:Optional + Internal *bool `json:"internal,omitempty"` } // +kubebuilder:object:generate=true diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index b9b64502..f61098ae 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -506,6 +506,11 @@ func (in *RoutingSpec) DeepCopyInto(out *RoutingSpec) { *out = new(bool) **out = **in } + if in.Internal != nil { + in, out := &in.Internal, &out.Internal + *out = new(bool) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RoutingSpec. diff --git a/config/crd/skiperator.kartverket.no_routings.yaml b/config/crd/skiperator.kartverket.no_routings.yaml index c6877df2..68a4c82b 100644 --- a/config/crd/skiperator.kartverket.no_routings.yaml +++ b/config/crd/skiperator.kartverket.no_routings.yaml @@ -45,6 +45,8 @@ spec: properties: hostname: type: string + internal: + type: boolean redirectToHTTPS: default: true type: boolean diff --git a/pkg/resourcegenerator/istio/gateway/routing.go b/pkg/resourcegenerator/istio/gateway/routing.go index 3f08edba..920d6bb2 100644 --- a/pkg/resourcegenerator/istio/gateway/routing.go +++ b/pkg/resourcegenerator/istio/gateway/routing.go @@ -28,6 +28,9 @@ func generateForRouting(r reconciliation.Reconciliation) error { } h, err := routing.Spec.GetHost() + if routing.Spec.Internal != nil { + h.Internal = *routing.Spec.Internal + } if err != nil { return err } diff --git a/pkg/resourcegenerator/networkpolicy/dynamic/routing.go b/pkg/resourcegenerator/networkpolicy/dynamic/routing.go index 1a525731..8650ee09 100644 --- a/pkg/resourcegenerator/networkpolicy/dynamic/routing.go +++ b/pkg/resourcegenerator/networkpolicy/dynamic/routing.go @@ -28,6 +28,9 @@ func generateForRouting(r reconciliation.Reconciliation) error { uniqueTargetApps[getNetworkPolicyName(routing, route.TargetApp)] = route } h, err := routing.Spec.GetHost() + if routing.Spec.Internal != nil { + h.Internal = *routing.Spec.Internal + } if err != nil { return err } From a2f5fb328b76e4fdbfbb67c9eade4031e8a05829 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A5rd=20Ove=20Hoel?= Date: Tue, 10 Sep 2024 10:28:44 +0200 Subject: [PATCH 19/34] newlines --- tests/routing/internal-flag/routing-internal-flag-assert.yaml | 2 +- tests/routing/internal-flag/routing-internal-flag.yaml | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/routing/internal-flag/routing-internal-flag-assert.yaml b/tests/routing/internal-flag/routing-internal-flag-assert.yaml index b950e200..c5018b9b 100644 --- a/tests/routing/internal-flag/routing-internal-flag-assert.yaml +++ b/tests/routing/internal-flag/routing-internal-flag-assert.yaml @@ -91,4 +91,4 @@ spec: app: istio-ingress-internal ports: - port: 9000 - protocol: TCP \ No newline at end of file + protocol: TCP diff --git a/tests/routing/internal-flag/routing-internal-flag.yaml b/tests/routing/internal-flag/routing-internal-flag.yaml index a1d123c3..6d75510d 100644 --- a/tests/routing/internal-flag/routing-internal-flag.yaml +++ b/tests/routing/internal-flag/routing-internal-flag.yaml @@ -12,4 +12,3 @@ spec: - pathPrefix: /new-path targetApp: app-2 rewriteUri: true - From e289bc367b3a86c1d4000bd0273a406b1f1fe9a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A5rd=20Ove=20Hoel?= Date: Tue, 10 Sep 2024 10:30:25 +0200 Subject: [PATCH 20/34] moved helperfunction --- pkg/resourcegenerator/istio/gateway/application.go | 3 ++- pkg/resourcegenerator/istio/gateway/routing.go | 2 +- pkg/resourcegenerator/networkpolicy/dynamic/routing.go | 3 ++- pkg/resourcegenerator/resourceutils/helpers.go | 8 ++++++++ pkg/util/helperfunctions.go | 8 -------- 5 files changed, 13 insertions(+), 11 deletions(-) diff --git a/pkg/resourcegenerator/istio/gateway/application.go b/pkg/resourcegenerator/istio/gateway/application.go index 39791107..c0e9aba6 100644 --- a/pkg/resourcegenerator/istio/gateway/application.go +++ b/pkg/resourcegenerator/istio/gateway/application.go @@ -4,6 +4,7 @@ import ( "fmt" skiperatorv1alpha1 "github.com/kartverket/skiperator/api/v1alpha1" "github.com/kartverket/skiperator/pkg/reconciliation" + "github.com/kartverket/skiperator/pkg/resourcegenerator/resourceutils" "github.com/kartverket/skiperator/pkg/util" networkingv1beta1api "istio.io/api/networking/v1beta1" networkingv1beta1 "istio.io/client-go/pkg/apis/networking/v1beta1" @@ -37,7 +38,7 @@ func generateForApplication(r reconciliation.Reconciliation) error { name := fmt.Sprintf("%s-ingress-%x", application.Name, util.GenerateHashFromName(h.Hostname)) gateway := networkingv1beta1.Gateway{ObjectMeta: metav1.ObjectMeta{Namespace: application.Namespace, Name: name}} - gateway.Spec.Selector = util.GetIstioGatewayLabelSelector(h) + gateway.Spec.Selector = resourceutils.GetIstioGatewayLabelSelector(h) gatewayServersToAdd := []*networkingv1beta1api.Server{} diff --git a/pkg/resourcegenerator/istio/gateway/routing.go b/pkg/resourcegenerator/istio/gateway/routing.go index 920d6bb2..e2382023 100644 --- a/pkg/resourcegenerator/istio/gateway/routing.go +++ b/pkg/resourcegenerator/istio/gateway/routing.go @@ -47,7 +47,7 @@ func generateForRouting(r reconciliation.Reconciliation) error { } } - gateway.Spec.Selector = util.GetIstioGatewayLabelSelector(h) + gateway.Spec.Selector = resourceutils.GetIstioGatewayLabelSelector(h) gateway.Spec.Servers = []*networkingv1beta1api.Server{ { Hosts: []string{h.Hostname}, diff --git a/pkg/resourcegenerator/networkpolicy/dynamic/routing.go b/pkg/resourcegenerator/networkpolicy/dynamic/routing.go index 8650ee09..64fd619d 100644 --- a/pkg/resourcegenerator/networkpolicy/dynamic/routing.go +++ b/pkg/resourcegenerator/networkpolicy/dynamic/routing.go @@ -5,6 +5,7 @@ import ( skiperatorv1alpha1 "github.com/kartverket/skiperator/api/v1alpha1" "github.com/kartverket/skiperator/pkg/reconciliation" + "github.com/kartverket/skiperator/pkg/resourcegenerator/resourceutils" "github.com/kartverket/skiperator/pkg/util" networkingv1 "k8s.io/api/networking/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -56,7 +57,7 @@ func generateForRouting(r reconciliation.Reconciliation) error { MatchLabels: util.GetIstioGatewaySelector(), }, PodSelector: &metav1.LabelSelector{ - MatchLabels: util.GetIstioGatewayLabelSelector(h), + MatchLabels: resourceutils.GetIstioGatewayLabelSelector(h), }, }, }, diff --git a/pkg/resourcegenerator/resourceutils/helpers.go b/pkg/resourcegenerator/resourceutils/helpers.go index d7bd46cf..8bfcb4a9 100644 --- a/pkg/resourcegenerator/resourceutils/helpers.go +++ b/pkg/resourcegenerator/resourceutils/helpers.go @@ -1,6 +1,7 @@ package resourceutils import ( + "github.com/kartverket/skiperator/api/v1alpha1" skiperatorv1alpha1 "github.com/kartverket/skiperator/api/v1alpha1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" ) @@ -16,3 +17,10 @@ func ShouldScaleToZero(jsonReplicas *apiextensionsv1.JSON) bool { } return false } + +func GetIstioGatewayLabelSelector(host *v1alpha1.Host) map[string]string { + if host.Internal { + return map[string]string{"app": "istio-ingress-internal"} + } + return map[string]string{"app": "istio-ingress-external"} +} diff --git a/pkg/util/helperfunctions.go b/pkg/util/helperfunctions.go index d0939183..9bd2ca4c 100644 --- a/pkg/util/helperfunctions.go +++ b/pkg/util/helperfunctions.go @@ -3,7 +3,6 @@ package util import ( "context" "fmt" - "github.com/kartverket/skiperator/api/v1alpha1" "github.com/kartverket/skiperator/api/v1alpha1/podtypes" "github.com/mitchellh/hashstructure/v2" "github.com/nais/liberator/pkg/namegen" @@ -17,13 +16,6 @@ import ( // TODO Clean up this file, move functions to more appropriate files -func GetIstioGatewayLabelSelector(host *v1alpha1.Host) map[string]string { - if host.Internal { - return map[string]string{"app": "istio-ingress-internal"} - } - return map[string]string{"app": "istio-ingress-external"} -} - func GetHashForStructs(obj []interface{}) string { hash, err := hashstructure.Hash(obj, hashstructure.FormatV2, nil) if err != nil { From ddb9785798a449db33466c36da8349067d4794a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A5rd=20Ove=20Hoel?= Date: Tue, 10 Sep 2024 10:32:11 +0200 Subject: [PATCH 21/34] errorhandling --- pkg/resourcegenerator/deployment/deployment.go | 1 + pkg/resourcegenerator/istio/gateway/routing.go | 9 ++++----- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/resourcegenerator/deployment/deployment.go b/pkg/resourcegenerator/deployment/deployment.go index e3a2cc9c..f7470430 100644 --- a/pkg/resourcegenerator/deployment/deployment.go +++ b/pkg/resourcegenerator/deployment/deployment.go @@ -207,6 +207,7 @@ func Generate(r reconciliation.Reconciliation) error { hosts, err := application.Spec.Hosts() if err != nil { ctxLog.Error(err, "could not get hosts from application") + return err } if deployment.Annotations == nil { deployment.Annotations = make(map[string]string) diff --git a/pkg/resourcegenerator/istio/gateway/routing.go b/pkg/resourcegenerator/istio/gateway/routing.go index e2382023..7212ef00 100644 --- a/pkg/resourcegenerator/istio/gateway/routing.go +++ b/pkg/resourcegenerator/istio/gateway/routing.go @@ -2,10 +2,9 @@ package gateway import ( "fmt" - skiperatorv1alpha1 "github.com/kartverket/skiperator/api/v1alpha1" "github.com/kartverket/skiperator/pkg/reconciliation" - "github.com/kartverket/skiperator/pkg/util" + "github.com/kartverket/skiperator/pkg/resourcegenerator/resourceutils" networkingv1beta1api "istio.io/api/networking/v1beta1" networkingv1beta1 "istio.io/client-go/pkg/apis/networking/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -28,12 +27,12 @@ func generateForRouting(r reconciliation.Reconciliation) error { } h, err := routing.Spec.GetHost() - if routing.Spec.Internal != nil { - h.Internal = *routing.Spec.Internal - } if err != nil { return err } + if routing.Spec.Internal != nil { + h.Internal = *routing.Spec.Internal + } gateway := networkingv1beta1.Gateway{ObjectMeta: metav1.ObjectMeta{Namespace: routing.Namespace, Name: routing.GetGatewayName()}} From a24ed0fab5257ace9b0a707b90c7d7b9b2d491e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A5rd=20Ove=20Hoel?= Date: Tue, 10 Sep 2024 10:33:09 +0200 Subject: [PATCH 22/34] rename variables --- api/v1alpha1/application_types.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/api/v1alpha1/application_types.go b/api/v1alpha1/application_types.go index 3a33563d..88eebbce 100644 --- a/api/v1alpha1/application_types.go +++ b/api/v1alpha1/application_types.go @@ -460,23 +460,23 @@ func MarshalledIngresses(Ingresses interface{}) *apiextensionsv1.JSON { } func (s *ApplicationSpec) Hosts() (HostCollection, error) { - var resultString []string - var resultObject []Ingresses + var ingressesAsString []string + var ingressesAsObject []Ingresses var errorsFound []error hosts := NewCollection() ingresses := MarshalledIngresses(s.Ingresses) - err := json.Unmarshal(ingresses.Raw, &resultString) + err := json.Unmarshal(ingresses.Raw, &ingressesAsString) if err != nil { - err := json.Unmarshal(ingresses.Raw, &resultObject) + err := json.Unmarshal(ingresses.Raw, &ingressesAsObject) if err != nil { errorsFound = append(errorsFound, err) return NewCollection(), errors.Join(errorsFound...) } - for _, ingress := range resultObject { - err := hosts.AddObject(ingress.Hostname, ingress.Internal) + for _, ingress := range ingressesAsObject { + err := hosts.Add(ingress.Hostname, ingress.Internal) if err != nil { errorsFound = append(errorsFound, err) return NewCollection(), errors.Join(errorsFound...) @@ -485,8 +485,8 @@ func (s *ApplicationSpec) Hosts() (HostCollection, error) { return hosts, nil } - for _, ingress := range resultString { - err := hosts.AddObject(ingress, IsInternal(ingress)) + for _, ingress := range ingressesAsString { + err := hosts.Add(ingress, IsInternal(ingress)) if err != nil { errorsFound = append(errorsFound, err) } From d9bd2abdee30a282ac610d97f6a64a982e682673 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A5rd=20Ove=20Hoel?= Date: Tue, 10 Sep 2024 10:34:22 +0200 Subject: [PATCH 23/34] moved type def and corrected typo --- api/v1alpha1/application_types.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/api/v1alpha1/application_types.go b/api/v1alpha1/application_types.go index 88eebbce..b96bd5b7 100644 --- a/api/v1alpha1/application_types.go +++ b/api/v1alpha1/application_types.go @@ -335,6 +335,11 @@ type PrometheusConfig struct { AllowAllMetrics bool `json:"allowAllMetrics,omitempty"` } +type Ingresses struct { + Hostname string + Internal bool +} + func NewDefaultReplicas() Replicas { return Replicas{ Min: 2, @@ -442,16 +447,11 @@ func (a *Application) GetCommonSpec() *CommonSpec { } } -type Ingresses struct { - Hostname string - Internal bool -} - -func MarshalledIngresses(Ingresses interface{}) *apiextensionsv1.JSON { +func MarshalledIngresses(ingresses interface{}) *apiextensionsv1.JSON { IngressesJSON := &apiextensionsv1.JSON{} var err error - IngressesJSON.Raw, err = json.Marshal(Ingresses) + IngressesJSON.Raw, err = json.Marshal(ingresses) if err == nil { return IngressesJSON } From 871a092197cce9ca561f548447a9cc77129715fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A5rd=20Ove=20Hoel?= Date: Tue, 10 Sep 2024 10:34:53 +0200 Subject: [PATCH 24/34] renamed function --- api/v1alpha1/hosts.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/v1alpha1/hosts.go b/api/v1alpha1/hosts.go index 9d21d1e8..eea2c827 100644 --- a/api/v1alpha1/hosts.go +++ b/api/v1alpha1/hosts.go @@ -68,7 +68,7 @@ func NewCollection() HostCollection { } } -func (hs *HostCollection) AddObject(hostname string, internal bool) error { +func (hs *HostCollection) Add(hostname string, internal bool) error { h, err := NewHost(hostname) if err != nil { return err From 44ae494afadca1840081e90378fdfcd3cff04a10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A5rd=20Ove=20Hoel?= Date: Mon, 16 Sep 2024 10:39:24 +0200 Subject: [PATCH 25/34] tests --- .../application-internal-flag-assert.yaml | 76 +++++++++++++++++++ .../application-internal-flag.yaml | 10 +++ ...internal-missing-internal-flag-assert.yaml | 76 +++++++++++++++++++ ...rnal-missing-internal-flag-ext-assert.yaml | 76 +++++++++++++++++++ ...on-internal-missing-internal-flag-ext.yaml | 9 +++ ...cation-internal-missing-internal-flag.yaml | 9 +++ .../internal-flag/chainsaw-test.yaml | 25 ++++++ .../routing/internal-flag/chainsaw-test.yaml | 3 +- 8 files changed, 282 insertions(+), 2 deletions(-) create mode 100644 tests/application/internal-flag/application-internal-flag-assert.yaml create mode 100644 tests/application/internal-flag/application-internal-flag.yaml create mode 100644 tests/application/internal-flag/application-internal-missing-internal-flag-assert.yaml create mode 100644 tests/application/internal-flag/application-internal-missing-internal-flag-ext-assert.yaml create mode 100644 tests/application/internal-flag/application-internal-missing-internal-flag-ext.yaml create mode 100644 tests/application/internal-flag/application-internal-missing-internal-flag.yaml create mode 100644 tests/application/internal-flag/chainsaw-test.yaml diff --git a/tests/application/internal-flag/application-internal-flag-assert.yaml b/tests/application/internal-flag/application-internal-flag-assert.yaml new file mode 100644 index 00000000..a1040c22 --- /dev/null +++ b/tests/application/internal-flag/application-internal-flag-assert.yaml @@ -0,0 +1,76 @@ +apiVersion: networking.istio.io/v1beta1 +kind: Gateway +metadata: + name: app-internal-flag-ingress-56cd7aa901014e78 +spec: + selector: + app: istio-ingress-internal + servers: + - hosts: + - example.com + port: + name: http + number: 80 + protocol: HTTP + - hosts: + - example.com + port: + name: https + number: 443 + protocol: HTTPS + tls: + credentialName: chainsaw-routing-internal-flag-app-internal-flag-ingress-56cd7aa901014e78 + mode: SIMPLE + +--- +apiVersion: networking.istio.io/v1beta1 +kind: VirtualService +metadata: + name: app-internal-flag-ingress +spec: + exportTo: + - . + - istio-system + - istio-gateways + gateways: + - app-internal-flag-ingress-56cd7aa901014e78 + hosts: + - example.com + 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: app-internal-flag + port: + number: 8000 +--- +apiVersion: networking.k8s.io/v1 +kind: NetworkPolicy +metadata: + name: app-internal-flag +spec: + ingress: + - from: + - namespaceSelector: + matchLabels: + kubernetes.io/metadata.name: istio-gateways + podSelector: + matchLabels: + app: istio-ingress-internal + ports: + - port: 8000 + protocol: TCP + podSelector: + matchLabels: + app: app-internal-flag + policyTypes: + - Ingress diff --git a/tests/application/internal-flag/application-internal-flag.yaml b/tests/application/internal-flag/application-internal-flag.yaml new file mode 100644 index 00000000..7c64d38f --- /dev/null +++ b/tests/application/internal-flag/application-internal-flag.yaml @@ -0,0 +1,10 @@ +apiVersion: skiperator.kartverket.no/v1alpha1 +kind: Application +metadata: + name: app-internal-flag +spec: + image: image + port: 8000 + ingresses: + - hostname: example.com + internal: true diff --git a/tests/application/internal-flag/application-internal-missing-internal-flag-assert.yaml b/tests/application/internal-flag/application-internal-missing-internal-flag-assert.yaml new file mode 100644 index 00000000..f64d59f0 --- /dev/null +++ b/tests/application/internal-flag/application-internal-missing-internal-flag-assert.yaml @@ -0,0 +1,76 @@ +apiVersion: networking.istio.io/v1beta1 +kind: Gateway +metadata: + name: app-missing-internal-flag-ingress-df26180a7fa049ff +spec: + selector: + app: istio-ingress-internal + servers: + - hosts: + - appmissinginternalflag.atkv3-dev.kartverket-intern.cloud + port: + name: http + number: 80 + protocol: HTTP + - hosts: + - appmissinginternalflag.atkv3-dev.kartverket-intern.cloud + port: + name: https + number: 443 + protocol: HTTPS + tls: + credentialName: chainsaw-routing-internal-flag-app-missing-internal-flag-ingress-df26180a7fa049ff + mode: SIMPLE + +--- +apiVersion: networking.istio.io/v1beta1 +kind: VirtualService +metadata: + name: app-missing-internal-flag-ingress +spec: + exportTo: + - . + - istio-system + - istio-gateways + gateways: + - app-missing-internal-flag-ingress-df26180a7fa049ff + hosts: + - appmissinginternalflag.atkv3-dev.kartverket-intern.cloud + 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: app-missing-internal-flag + port: + number: 8000 +--- +apiVersion: networking.k8s.io/v1 +kind: NetworkPolicy +metadata: + name: app-missing-internal-flag +spec: + ingress: + - from: + - namespaceSelector: + matchLabels: + kubernetes.io/metadata.name: istio-gateways + podSelector: + matchLabels: + app: istio-ingress-internal + ports: + - port: 8000 + protocol: TCP + podSelector: + matchLabels: + app: app-missing-internal-flag + policyTypes: + - Ingress diff --git a/tests/application/internal-flag/application-internal-missing-internal-flag-ext-assert.yaml b/tests/application/internal-flag/application-internal-missing-internal-flag-ext-assert.yaml new file mode 100644 index 00000000..2ce8a2e6 --- /dev/null +++ b/tests/application/internal-flag/application-internal-missing-internal-flag-ext-assert.yaml @@ -0,0 +1,76 @@ +apiVersion: networking.istio.io/v1beta1 +kind: Gateway +metadata: + name: app-missing-internal-flag-ingress-56cd7aa901014e78 +spec: + selector: + app: istio-ingress-external + servers: + - hosts: + - example.com + port: + name: http + number: 80 + protocol: HTTP + - hosts: + - example.com + port: + name: https + number: 443 + protocol: HTTPS + tls: + credentialName: chainsaw-routing-internal-flag-app-missing-internal-flag-ingress-56cd7aa901014e78 + mode: SIMPLE + +--- +apiVersion: networking.istio.io/v1beta1 +kind: VirtualService +metadata: + name: app-missing-internal-flag-ingress +spec: + exportTo: + - . + - istio-system + - istio-gateways + gateways: + - app-missing-internal-flag-ingress-56cd7aa901014e78 + hosts: + - example.com + 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: app-missing-internal-flag + port: + number: 8000 +--- +apiVersion: networking.k8s.io/v1 +kind: NetworkPolicy +metadata: + name: app-missing-internal-flag +spec: + ingress: + - from: + - namespaceSelector: + matchLabels: + kubernetes.io/metadata.name: istio-gateways + podSelector: + matchLabels: + app: istio-ingress-external + ports: + - port: 8000 + protocol: TCP + podSelector: + matchLabels: + app: app-missing-internal-flag + policyTypes: + - Ingress diff --git a/tests/application/internal-flag/application-internal-missing-internal-flag-ext.yaml b/tests/application/internal-flag/application-internal-missing-internal-flag-ext.yaml new file mode 100644 index 00000000..05d0305e --- /dev/null +++ b/tests/application/internal-flag/application-internal-missing-internal-flag-ext.yaml @@ -0,0 +1,9 @@ +apiVersion: skiperator.kartverket.no/v1alpha1 +kind: Application +metadata: + name: app-missing-internal-flag +spec: + image: image + port: 8000 + ingresses: + - hostname: example.com diff --git a/tests/application/internal-flag/application-internal-missing-internal-flag.yaml b/tests/application/internal-flag/application-internal-missing-internal-flag.yaml new file mode 100644 index 00000000..9c795d31 --- /dev/null +++ b/tests/application/internal-flag/application-internal-missing-internal-flag.yaml @@ -0,0 +1,9 @@ +apiVersion: skiperator.kartverket.no/v1alpha1 +kind: Application +metadata: + name: app-missing-internal-flag +spec: + image: image + port: 8000 + ingresses: + - hostname: appmissinginternalflag.atkv3-dev.kartverket-intern.cloud diff --git a/tests/application/internal-flag/chainsaw-test.yaml b/tests/application/internal-flag/chainsaw-test.yaml new file mode 100644 index 00000000..cfcb18e4 --- /dev/null +++ b/tests/application/internal-flag/chainsaw-test.yaml @@ -0,0 +1,25 @@ +apiVersion: chainsaw.kyverno.io/v1alpha1 +kind: Test +metadata: + name: routes +spec: + skip: false + concurrent: true + skipDelete: false + namespace: chainsaw-routing-internal-flag + steps: + - try: + - apply: + file: application-internal-flag.yaml + - assert: + file: application-internal-flag-assert.yaml + - try: + - apply: + file: application-internal-missing-internal-flag.yaml + - assert: + file: application-internal-missing-internal-flag-assert.yaml + - try: + - patch: + file: application-internal-missing-internal-flag-ext.yaml + - assert: + file: application-internal-missing-internal-flag-ext-assert.yaml diff --git a/tests/routing/internal-flag/chainsaw-test.yaml b/tests/routing/internal-flag/chainsaw-test.yaml index fb9dea17..5bb01f61 100644 --- a/tests/routing/internal-flag/chainsaw-test.yaml +++ b/tests/routing/internal-flag/chainsaw-test.yaml @@ -13,6 +13,5 @@ spec: file: application.yaml - apply: file: routing-internal-flag.yaml - - assert: - file: routing-internal-flag-assert.yaml + file: routing-internal-flag-assert.yaml \ No newline at end of file From 653f7406648f2107924b612a6e7e84dd74cc0bfc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A5rd=20Ove=20Hoel?= Date: Mon, 16 Sep 2024 10:40:40 +0200 Subject: [PATCH 26/34] pointer bool to enable nil check. --- api/v1alpha1/application_types.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/api/v1alpha1/application_types.go b/api/v1alpha1/application_types.go index b96bd5b7..fb4db8d8 100644 --- a/api/v1alpha1/application_types.go +++ b/api/v1alpha1/application_types.go @@ -6,6 +6,7 @@ import ( "github.com/kartverket/skiperator/api/v1alpha1/digdirator" "github.com/kartverket/skiperator/api/v1alpha1/istiotypes" "github.com/kartverket/skiperator/api/v1alpha1/podtypes" + "github.com/kartverket/skiperator/pkg/util" corev1 "k8s.io/api/core/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -337,7 +338,7 @@ type PrometheusConfig struct { type Ingresses struct { Hostname string - Internal bool + Internal *bool } func NewDefaultReplicas() Replicas { @@ -476,7 +477,11 @@ func (s *ApplicationSpec) Hosts() (HostCollection, error) { } for _, ingress := range ingressesAsObject { - err := hosts.Add(ingress.Hostname, ingress.Internal) + if ingress.Internal == nil { + ingress.Internal = util.PointTo(IsInternal(ingress.Hostname)) + } + + err := hosts.Add(ingress.Hostname, *ingress.Internal) if err != nil { errorsFound = append(errorsFound, err) return NewCollection(), errors.Join(errorsFound...) From 0b54024c076d8531e86554fad592d08982694b65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A5rd=20Ove=20Hoel?= Date: Wed, 18 Sep 2024 13:32:10 +0200 Subject: [PATCH 27/34] Refactor tests. --- api/v1alpha1/application_types.go | 12 ------ ...skiperator.kartverket.no_applications.yaml | 4 +- tests/routing/internal-flag/application.yaml | 8 ++-- .../routing/internal-flag/chainsaw-test.yaml | 4 +- .../routing-internal-flag-assert.yaml | 42 ++++++++++++++----- .../internal-flag/routing-internal-flag.yaml | 6 +-- 6 files changed, 42 insertions(+), 34 deletions(-) diff --git a/api/v1alpha1/application_types.go b/api/v1alpha1/application_types.go index 7c3a1325..fb4db8d8 100644 --- a/api/v1alpha1/application_types.go +++ b/api/v1alpha1/application_types.go @@ -460,18 +460,6 @@ func MarshalledIngresses(ingresses interface{}) *apiextensionsv1.JSON { return nil } -func MarshalledIngresses(ingresses interface{}) *apiextensionsv1.JSON { - IngressesJSON := &apiextensionsv1.JSON{} - var err error - - IngressesJSON.Raw, err = json.Marshal(ingresses) - if err == nil { - return IngressesJSON - } - - return nil -} - func (s *ApplicationSpec) Hosts() (HostCollection, error) { var ingressesAsString []string var ingressesAsObject []Ingresses diff --git a/config/crd/skiperator.kartverket.no_applications.yaml b/config/crd/skiperator.kartverket.no_applications.yaml index e14da360..26a9572a 100644 --- a/config/crd/skiperator.kartverket.no_applications.yaml +++ b/config/crd/skiperator.kartverket.no_applications.yaml @@ -646,9 +646,7 @@ spec: Ingresses must be lowercase, contain no spaces, be a non-empty string, and have a hostname/domain separated by a period They can optionally be suffixed with a plus and name of a custom TLS secret located in the istio-gateways namespace. E.g. "foo.atkv3-dev.kartverket-intern.cloud+env-wildcard-cert" - items: - type: string - type: array + x-kubernetes-preserve-unknown-fields: true istioSettings: default: telemetry: diff --git a/tests/routing/internal-flag/application.yaml b/tests/routing/internal-flag/application.yaml index 32c89a0f..64637532 100644 --- a/tests/routing/internal-flag/application.yaml +++ b/tests/routing/internal-flag/application.yaml @@ -1,15 +1,15 @@ apiVersion: skiperator.kartverket.no/v1alpha1 kind: Application metadata: - name: app-1 + name: internal-flag-app1 spec: - image: gcr.io/google-samples/istio/helloserver:v0.0.1 + image: image port: 8000 --- apiVersion: skiperator.kartverket.no/v1alpha1 kind: Application metadata: - name: app-2 + name: internal-flag-app2 spec: - image: gcr.io/google-samples/istio/helloserver:v0.0.1 + image: image port: 9000 diff --git a/tests/routing/internal-flag/chainsaw-test.yaml b/tests/routing/internal-flag/chainsaw-test.yaml index 5bb01f61..031c9998 100644 --- a/tests/routing/internal-flag/chainsaw-test.yaml +++ b/tests/routing/internal-flag/chainsaw-test.yaml @@ -1,10 +1,10 @@ apiVersion: chainsaw.kyverno.io/v1alpha1 kind: Test metadata: - name: routes + name: routes-internal-flag spec: skip: false - concurrent: true + concurrent: false skipDelete: false namespace: chainsaw-routing-internal-flag steps: diff --git a/tests/routing/internal-flag/routing-internal-flag-assert.yaml b/tests/routing/internal-flag/routing-internal-flag-assert.yaml index c5018b9b..eb7bc159 100644 --- a/tests/routing/internal-flag/routing-internal-flag-assert.yaml +++ b/tests/routing/internal-flag/routing-internal-flag-assert.yaml @@ -1,7 +1,7 @@ apiVersion: networking.istio.io/v1beta1 kind: Gateway metadata: - name: app-paths-routing-ingress + name: app-paths-internal-flag-routing-ingress spec: selector: app: istio-ingress-internal @@ -19,21 +19,21 @@ spec: number: 443 protocol: HTTPS tls: - credentialName: chainsaw-routing-internal-flag-app-paths-routing-ingre-9441fe86 + credentialName: chainsaw-routing-internal-flag-app-paths-internal-flag-67aee041 mode: SIMPLE --- apiVersion: networking.istio.io/v1beta1 kind: VirtualService metadata: - name: app-paths-routing-ingress + name: app-paths-internal-flag-routing-ingress spec: exportTo: - . - istio-system - istio-gateways gateways: - - app-paths-routing-ingress + - app-paths-internal-flag-routing-ingress hosts: - example.com http: @@ -52,10 +52,10 @@ spec: prefix: /app1 rewrite: uri: / - name: app-1 + name: internal-flag-app1 route: - destination: - host: app-1 + host: internal-flag-app1 port: number: 8000 - match: @@ -64,21 +64,21 @@ spec: prefix: /new-path rewrite: uri: / - name: app-2 + name: internal-flag-app2 route: - destination: - host: app-2 + host: internal-flag-app2 port: number: 9000 --- apiVersion: networking.k8s.io/v1 kind: NetworkPolicy metadata: - name: app-paths-app-2-istio-ingress + name: app-paths-internal-flag-internal-flag-app2-istio-ingress spec: podSelector: matchLabels: - app: app-2 + app: internal-flag-app2 policyTypes: - Ingress ingress: @@ -92,3 +92,25 @@ spec: ports: - port: 9000 protocol: TCP +--- +apiVersion: networking.k8s.io/v1 +kind: NetworkPolicy +metadata: + name: app-paths-internal-flag-internal-flag-app1-istio-ingress +spec: + ingress: + - from: + - namespaceSelector: + matchLabels: + kubernetes.io/metadata.name: istio-gateways + podSelector: + matchLabels: + app: istio-ingress-internal + ports: + - port: 8000 + protocol: TCP + podSelector: + matchLabels: + app: internal-flag-app1 + policyTypes: + - Ingress diff --git a/tests/routing/internal-flag/routing-internal-flag.yaml b/tests/routing/internal-flag/routing-internal-flag.yaml index 6d75510d..3f8a319a 100644 --- a/tests/routing/internal-flag/routing-internal-flag.yaml +++ b/tests/routing/internal-flag/routing-internal-flag.yaml @@ -1,14 +1,14 @@ apiVersion: skiperator.kartverket.no/v1alpha1 kind: Routing metadata: - name: app-paths + name: app-paths-internal-flag spec: hostname: example.com internal: true routes: - pathPrefix: /app1 - targetApp: app-1 + targetApp: internal-flag-app1 rewriteUri: true - pathPrefix: /new-path - targetApp: app-2 + targetApp: internal-flag-app2 rewriteUri: true From 9802c879dbb5b149e2a0e12b72d704b56d8ae855 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A5rd=20Ove=20Hoel?= <93327610+BardOve@users.noreply.github.com> Date: Fri, 20 Sep 2024 14:22:46 +0200 Subject: [PATCH 28/34] Update api/v1alpha1/hosts.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Martin Haram Nygård --- api/v1alpha1/hosts.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/v1alpha1/hosts.go b/api/v1alpha1/hosts.go index eea2c827..8bb9372a 100644 --- a/api/v1alpha1/hosts.go +++ b/api/v1alpha1/hosts.go @@ -111,6 +111,6 @@ func (hs *HostCollection) Count() int { return len(hs.hosts) } -func IsInternal(hostname string) bool { +func isInternal(hostname string) bool { return internalPattern.MatchString(hostname) } From 6cbaad4fbd8b53f0d788a2e0ada7e3d8a96906e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A5rd=20Ove=20Hoel?= Date: Fri, 20 Sep 2024 15:23:42 +0200 Subject: [PATCH 29/34] rename function --- api/v1alpha1/application_types.go | 4 ++-- api/v1alpha1/hosts.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/api/v1alpha1/application_types.go b/api/v1alpha1/application_types.go index fb4db8d8..3ca3e21c 100644 --- a/api/v1alpha1/application_types.go +++ b/api/v1alpha1/application_types.go @@ -478,7 +478,7 @@ func (s *ApplicationSpec) Hosts() (HostCollection, error) { for _, ingress := range ingressesAsObject { if ingress.Internal == nil { - ingress.Internal = util.PointTo(IsInternal(ingress.Hostname)) + ingress.Internal = util.PointTo(isInternal(ingress.Hostname)) } err := hosts.Add(ingress.Hostname, *ingress.Internal) @@ -491,7 +491,7 @@ func (s *ApplicationSpec) Hosts() (HostCollection, error) { } for _, ingress := range ingressesAsString { - err := hosts.Add(ingress, IsInternal(ingress)) + err := hosts.Add(ingress, isInternal(ingress)) if err != nil { errorsFound = append(errorsFound, err) } diff --git a/api/v1alpha1/hosts.go b/api/v1alpha1/hosts.go index 8bb9372a..be396a24 100644 --- a/api/v1alpha1/hosts.go +++ b/api/v1alpha1/hosts.go @@ -54,7 +54,7 @@ func NewHost(hostname string) (*Host, error) { if err := domain.Check(h.Hostname); err != nil { return nil, fmt.Errorf("%s: failed validation: %w", h.Hostname, err) } - h.Internal = IsInternal(hostname) + h.Internal = isInternal(hostname) return &h, nil } From 49b3f7e6bf82c0dcfa0468809ea810cdf19a8a99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A5rd=20Ove=20Hoel?= Date: Fri, 27 Sep 2024 10:54:50 +0200 Subject: [PATCH 30/34] rename app and resources in tests --- ...-internal-missing-internal-flag-ext-assert.yaml | 14 +++++++------- ...ication-internal-missing-internal-flag-ext.yaml | 2 +- tests/application/internal-flag/chainsaw-test.yaml | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/application/internal-flag/application-internal-missing-internal-flag-ext-assert.yaml b/tests/application/internal-flag/application-internal-missing-internal-flag-ext-assert.yaml index 2ce8a2e6..2cd6c7ca 100644 --- a/tests/application/internal-flag/application-internal-missing-internal-flag-ext-assert.yaml +++ b/tests/application/internal-flag/application-internal-missing-internal-flag-ext-assert.yaml @@ -1,7 +1,7 @@ apiVersion: networking.istio.io/v1beta1 kind: Gateway metadata: - name: app-missing-internal-flag-ingress-56cd7aa901014e78 + name: app-missing-internal-flag-ext-ingress-56cd7aa901014e78 spec: selector: app: istio-ingress-external @@ -19,21 +19,21 @@ spec: number: 443 protocol: HTTPS tls: - credentialName: chainsaw-routing-internal-flag-app-missing-internal-flag-ingress-56cd7aa901014e78 + credentialName: chainsaw-routing-internal-flag-app-missing-internal-flag-ext-ingress-56cd7aa901014e78 mode: SIMPLE --- apiVersion: networking.istio.io/v1beta1 kind: VirtualService metadata: - name: app-missing-internal-flag-ingress + name: app-missing-internal-flag-ext-ingress spec: exportTo: - . - istio-system - istio-gateways gateways: - - app-missing-internal-flag-ingress-56cd7aa901014e78 + - app-missing-internal-flag-ext-ingress-56cd7aa901014e78 hosts: - example.com http: @@ -49,14 +49,14 @@ spec: - name: default-app-route route: - destination: - host: app-missing-internal-flag + host: app-missing-internal-flag-ext port: number: 8000 --- apiVersion: networking.k8s.io/v1 kind: NetworkPolicy metadata: - name: app-missing-internal-flag + name: app-missing-internal-flag-ext spec: ingress: - from: @@ -71,6 +71,6 @@ spec: protocol: TCP podSelector: matchLabels: - app: app-missing-internal-flag + app: app-missing-internal-flag-ext policyTypes: - Ingress diff --git a/tests/application/internal-flag/application-internal-missing-internal-flag-ext.yaml b/tests/application/internal-flag/application-internal-missing-internal-flag-ext.yaml index 05d0305e..0476049c 100644 --- a/tests/application/internal-flag/application-internal-missing-internal-flag-ext.yaml +++ b/tests/application/internal-flag/application-internal-missing-internal-flag-ext.yaml @@ -1,7 +1,7 @@ apiVersion: skiperator.kartverket.no/v1alpha1 kind: Application metadata: - name: app-missing-internal-flag + name: app-missing-internal-flag-ext spec: image: image port: 8000 diff --git a/tests/application/internal-flag/chainsaw-test.yaml b/tests/application/internal-flag/chainsaw-test.yaml index cfcb18e4..7e9debc4 100644 --- a/tests/application/internal-flag/chainsaw-test.yaml +++ b/tests/application/internal-flag/chainsaw-test.yaml @@ -19,7 +19,7 @@ spec: - assert: file: application-internal-missing-internal-flag-assert.yaml - try: - - patch: + - apply: file: application-internal-missing-internal-flag-ext.yaml - assert: file: application-internal-missing-internal-flag-ext-assert.yaml From 9e0e7906210158edaaae453423e55f2aebb3783a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A5rd=20Ove=20Hoel?= Date: Mon, 30 Sep 2024 09:53:39 +0200 Subject: [PATCH 31/34] Avoid error shadowing. --- api/v1alpha1/application_types.go | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/api/v1alpha1/application_types.go b/api/v1alpha1/application_types.go index 3ca3e21c..4c1b962b 100644 --- a/api/v1alpha1/application_types.go +++ b/api/v1alpha1/application_types.go @@ -469,10 +469,10 @@ func (s *ApplicationSpec) Hosts() (HostCollection, error) { ingresses := MarshalledIngresses(s.Ingresses) err := json.Unmarshal(ingresses.Raw, &ingressesAsString) + // If the ingress supplied is not a string, we assume it's an object if err != nil { - err := json.Unmarshal(ingresses.Raw, &ingressesAsObject) - if err != nil { - errorsFound = append(errorsFound, err) + if mErr := json.Unmarshal(ingresses.Raw, &ingressesAsObject); mErr != nil { + errorsFound = append(errorsFound, mErr) return NewCollection(), errors.Join(errorsFound...) } @@ -481,15 +481,14 @@ func (s *ApplicationSpec) Hosts() (HostCollection, error) { ingress.Internal = util.PointTo(isInternal(ingress.Hostname)) } - err := hosts.Add(ingress.Hostname, *ingress.Internal) - if err != nil { - errorsFound = append(errorsFound, err) - return NewCollection(), errors.Join(errorsFound...) + if aErr := hosts.Add(ingress.Hostname, *ingress.Internal); aErr != nil { + errorsFound = append(errorsFound, aErr) } } - return hosts, nil + return hosts, errors.Join(errorsFound...) } + //Handle legacy ingress format for _, ingress := range ingressesAsString { err := hosts.Add(ingress, isInternal(ingress)) if err != nil { From 12eb2a80006a0a238b05f229532b77f82fc71054 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A5rd=20Ove=20Hoel?= Date: Mon, 30 Sep 2024 15:14:59 +0200 Subject: [PATCH 32/34] Avoid error shadowing. --- api/v1alpha1/application_types.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/v1alpha1/application_types.go b/api/v1alpha1/application_types.go index 4c1b962b..a90adb4c 100644 --- a/api/v1alpha1/application_types.go +++ b/api/v1alpha1/application_types.go @@ -490,8 +490,8 @@ func (s *ApplicationSpec) Hosts() (HostCollection, error) { //Handle legacy ingress format for _, ingress := range ingressesAsString { - err := hosts.Add(ingress, isInternal(ingress)) - if err != nil { + bErr := hosts.Add(ingress, isInternal(ingress)) + if bErr != nil { errorsFound = append(errorsFound, err) } } From 6399cbd4061410932e3807cf6d29904b2439faf5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A5rd=20Ove=20Hoel?= Date: Wed, 2 Oct 2024 07:27:04 +0200 Subject: [PATCH 33/34] Revert "Avoid error shadowing." This reverts commit 12eb2a80006a0a238b05f229532b77f82fc71054. --- api/v1alpha1/application_types.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/v1alpha1/application_types.go b/api/v1alpha1/application_types.go index a90adb4c..4c1b962b 100644 --- a/api/v1alpha1/application_types.go +++ b/api/v1alpha1/application_types.go @@ -490,8 +490,8 @@ func (s *ApplicationSpec) Hosts() (HostCollection, error) { //Handle legacy ingress format for _, ingress := range ingressesAsString { - bErr := hosts.Add(ingress, isInternal(ingress)) - if bErr != nil { + err := hosts.Add(ingress, isInternal(ingress)) + if err != nil { errorsFound = append(errorsFound, err) } } From 01d9c3056b6a2ec9f1e8a14fc857489bccc4ff46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A5rd=20Ove=20Hoel?= Date: Tue, 8 Oct 2024 08:41:49 +0200 Subject: [PATCH 34/34] cleanup --- api/v1alpha1/application_types.go | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/api/v1alpha1/application_types.go b/api/v1alpha1/application_types.go index 4c1b962b..7d17c04a 100644 --- a/api/v1alpha1/application_types.go +++ b/api/v1alpha1/application_types.go @@ -3,6 +3,7 @@ package v1alpha1 import ( "encoding/json" "errors" + "fmt" "github.com/kartverket/skiperator/api/v1alpha1/digdirator" "github.com/kartverket/skiperator/api/v1alpha1/istiotypes" "github.com/kartverket/skiperator/api/v1alpha1/podtypes" @@ -336,7 +337,7 @@ type PrometheusConfig struct { AllowAllMetrics bool `json:"allowAllMetrics,omitempty"` } -type Ingresses struct { +type Ingress struct { Hostname string Internal *bool } @@ -448,29 +449,31 @@ func (a *Application) GetCommonSpec() *CommonSpec { } } -func MarshalledIngresses(ingresses interface{}) *apiextensionsv1.JSON { - IngressesJSON := &apiextensionsv1.JSON{} +func MarshalledIngresses(ingresses interface{}) (*apiextensionsv1.JSON, error) { + marshalled := &apiextensionsv1.JSON{} var err error - IngressesJSON.Raw, err = json.Marshal(ingresses) - if err == nil { - return IngressesJSON + if marshalled.Raw, err = json.Marshal(ingresses); err != nil { + return nil, fmt.Errorf("could not marshal ingresses: %w", err) } - return nil + return marshalled, nil } func (s *ApplicationSpec) Hosts() (HostCollection, error) { var ingressesAsString []string - var ingressesAsObject []Ingresses + var ingressesAsObject []Ingress var errorsFound []error hosts := NewCollection() - ingresses := MarshalledIngresses(s.Ingresses) - err := json.Unmarshal(ingresses.Raw, &ingressesAsString) + ingresses, err := MarshalledIngresses(s.Ingresses) + if err != nil { + return hosts, err + } + aErr := json.Unmarshal(ingresses.Raw, &ingressesAsString) // If the ingress supplied is not a string, we assume it's an object - if err != nil { + if aErr != nil { if mErr := json.Unmarshal(ingresses.Raw, &ingressesAsObject); mErr != nil { errorsFound = append(errorsFound, mErr) return NewCollection(), errors.Join(errorsFound...)