From a2204cc4262409c17159dddee776fd014ad38397 Mon Sep 17 00:00:00 2001 From: mzeevi Date: Tue, 4 Jun 2024 11:51:15 +0000 Subject: [PATCH] change prepareResource for domainMapping, aRecordSet and Certificate With this PR, the name and fields of the DomainMapping, ARecordSet and Certificate objects are all based on the provided DNS Zone (parsed now from a ConfigMap) and not only on the Capp Route Hostname. This allows making sure that the resources are created for the correct DNS Zone, even if the hostname does not explicitly indicates the Zone. The tests have been changed and in addition the README.md has been modified. Signed-off-by: mzeevi --- README.md | 28 ++++++-- config/default/kustomization.yaml | 16 ----- config/manager/manager.yaml | 3 - .../kinds/capp/resourcemanagers/arecordset.go | 25 ++------ .../capp/resourcemanagers/certificate.go | 25 ++++++-- .../capp/resourcemanagers/domainmapping.go | 20 ++++-- internal/kinds/capp/resourcemanagers/ksvc.go | 7 +- internal/kinds/capp/status/route.go | 25 +++++--- internal/kinds/capp/utils/route.go | 64 +++++++++++++++++++ internal/kinds/capp/utils/utils.go | 2 + test/e2e_tests/arecordset_e2e_test.go | 9 ++- test/e2e_tests/certificate_e2e_test.go | 16 +++-- test/e2e_tests/domain_mapping_e2e_test.go | 15 +++-- test/e2e_tests/knative_e2e_test.go | 2 +- test/e2e_tests/mocks/base.go | 3 + test/e2e_tests/mocks/certificate.go | 16 ----- test/e2e_tests/mocks/dnsendpoint.go | 15 ----- test/e2e_tests/mocks/domain_mapping.go | 16 ----- test/e2e_tests/mocks/route.go | 37 +++++++++++ test/e2e_tests/suite_test.go | 28 +++++++- test/e2e_tests/testconsts/testconsts.go | 7 +- test/e2e_tests/utils/resources_adpater.go | 2 +- test/e2e_tests/utils/route_adapter.go | 16 +++++ 23 files changed, 264 insertions(+), 133 deletions(-) create mode 100644 internal/kinds/capp/utils/route.go delete mode 100644 test/e2e_tests/mocks/certificate.go delete mode 100644 test/e2e_tests/mocks/dnsendpoint.go delete mode 100644 test/e2e_tests/mocks/domain_mapping.go create mode 100644 test/e2e_tests/mocks/route.go diff --git a/README.md b/README.md index cdc3e4b5..86e00c54 100644 --- a/README.md +++ b/README.md @@ -45,15 +45,17 @@ The `container-app-operator` project can work as a standalone solution, but is m 1. A Kubernetes cluster (you can [use KinD](https://kind.sigs.k8s.io/docs/user/quick-start/)). -2. `Knative Serving` installed on the cluster (you can [use the quickstart](https://knative.dev/docs/getting-started/quickstart-install/)). +2. `knative-serving` installed on the cluster (you can [use the quickstart](https://knative.dev/docs/getting-started/quickstart-install/)). -3. `NFSPVC Operator` installed on the cluster (you can [use the `install.yaml`](https://github.com/dana-team/nfspvc-operator/releases)). +3. `nfspvc-operator` installed on the cluster (you can [use the `install.yaml`](https://github.com/dana-team/nfspvc-operator/releases)). -4. `External DNS` installed on the cluster (you can [use the Helm Chart](https://artifacthub.io/packages/helm/external-dns/external-dns)). +4. `provider-dns` and `Crossplane` installed on the cluster (you can [follow the instructions](https://github.com/dana-team/provider-dns) for the provider and [for Crossplane](https://docs.crossplane.io/latest/software/install/)). -5. `Logging Operator` installed on the cluster (you can [use the Helm Chart](https://kube-logging.dev/docs/install/#deploy-logging-operator-with-helm)). +5. `certificate-operator` installed on the cluster (you can [use the `install.yaml`](https://github.com/dana-team/certificate-operator/releases)) -`Knative Serving`, `Logging Operator`, `NFSPVC Operator` and `External DNS` can also be installed by running: +5. `logging-operator` installed on the cluster (you can [use the Helm Chart](https://kube-logging.dev/docs/install/#deploy-logging-operator-with-helm)). + +Everything can also be installed by running: ```bash $ make prereq @@ -109,6 +111,22 @@ It's possible to use the following one-liner: $ kubectl patch --namespace knative-serving configmap/config-features --type merge --patch '{"data":{"kubernetes.podspec-persistent-volume-claim": "enabled", "kubernetes.podspec-persistent-volume-write": "enabled"}}' ``` +### Using a Custom Hostname + +`Capp` enables using a custom hostname for the application. This in turn creates `DomainMapping` and `ARecordSet` objects and a `Certificate` object if `TLS` is desired. + +To correctly create the resources, it is needed to provider the operator with the `DNS Zone` where the application is exposed. This is done using a `ConfigMap` called `dns-zone` which needs to be created in the operator namespace. Note the trailing `.` which must be added to the zone name: + +```yaml +kind: ConfigMap +apiVersion: v1 +metadata: + name: dns-zone + namespace: capp-operator-system +data: + zone: "capp-zone.com." +``` + ## Example Capp ```yaml diff --git a/config/default/kustomization.yaml b/config/default/kustomization.yaml index 057b714c..15dbc17b 100644 --- a/config/default/kustomization.yaml +++ b/config/default/kustomization.yaml @@ -41,24 +41,8 @@ patches: # 'CERTMANAGER' needs to be enabled to use ca injection #- path: webhookcainjection_patch.yaml -configMapGenerator: - - name: dns-zone - literals: - - zone="dana-team-zone." - # [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER' prefix. # Uncomment the following replacements to add the cert-manager CA injection annotations -replacements: - - source: - kind: ConfigMap - name: dns-zone - fieldPath: data.zone - targets: - - select: - kind: Deployment - name: controller-manager - fieldPaths: - - spec.template.spec.containers.[name=manager].env.[name=CAPP_DNS_ZONE].value # - source: # Add cert-manager annotation to ValidatingWebhookConfiguration, MutatingWebhookConfiguration and CRDs # kind: Certificate # group: cert-manager.io diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml index 5abd6154..2eb0b6d4 100644 --- a/config/manager/manager.yaml +++ b/config/manager/manager.yaml @@ -90,9 +90,6 @@ spec: port: 8081 initialDelaySeconds: 5 periodSeconds: 10 - env: - - name: CAPP_DNS_ZONE - value: $(ZONE) # TODO(user): Configure the resources accordingly based on the project requirements. # More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ resources: diff --git a/internal/kinds/capp/resourcemanagers/arecordset.go b/internal/kinds/capp/resourcemanagers/arecordset.go index a3ab72d7..e69e333c 100644 --- a/internal/kinds/capp/resourcemanagers/arecordset.go +++ b/internal/kinds/capp/resourcemanagers/arecordset.go @@ -4,9 +4,9 @@ import ( "context" "fmt" "net" - "os" "reflect" + "github.com/dana-team/container-app-operator/internal/kinds/capp/utils" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/selection" @@ -29,7 +29,6 @@ const ( eventCappARecordSetCreationFailed = "ARecordSetCreationFailed" eventCappARecordSetCreated = "ARecordSetCreated" cappNamespaceKey = "rcs.dana.io/parent-capp-ns" - zoneEnvKey = "CAPP_DNS_ZONE" xpProviderDNSConfigRefName = "dns-default" localhostAddress = "127.0.0.1" ) @@ -41,18 +40,6 @@ type ARecordSetManager struct { EventRecorder record.EventRecorder } -// getZoneForRecord returns the zone to be used for the record from an environment variable. -func getZoneForRecord() (string, error) { - zone, ok := os.LookupEnv(zoneEnvKey) - if !ok { - return zone, fmt.Errorf("%s environment variable not set", zoneEnvKey) - } else if zone == "" { - return zone, fmt.Errorf("%s environment variable is empty", zoneEnvKey) - } - - return zone, nil -} - // getAddressesForRecord performs a DNS lookup for the given URL and returns a slice of IP address strings. func getAddressesForRecord(url string) ([]*string, error) { ips, err := net.LookupIP(url) @@ -74,12 +61,14 @@ func (r ARecordSetManager) prepareResource(capp cappv1alpha1.Capp) (dnsv1alpha1. placeholderAddress := localhostAddress addresses := []*string{&placeholderAddress} - name := capp.Spec.RouteSpec.Hostname - zone, err := getZoneForRecord() + zone, err := utils.GetZoneFromConfig(r.Ctx, r.K8sclient) if err != nil { return dnsv1alpha1.ARecordSet{}, err } + resourceName := utils.GenerateResourceName(capp.Spec.RouteSpec.Hostname, zone) + recordName := utils.GenerateRecordName(capp.Spec.RouteSpec.Hostname, zone) + // in a normal behavior, it can be assumed that this condition will eventually be satisfied // since there will be another reconciliation loop after the Capp status is updated if capp.Status.KnativeObjectStatus.URL != nil { @@ -92,7 +81,7 @@ func (r ARecordSetManager) prepareResource(capp cappv1alpha1.Capp) (dnsv1alpha1. recordset := dnsv1alpha1.ARecordSet{ TypeMeta: metav1.TypeMeta{}, ObjectMeta: metav1.ObjectMeta{ - Name: capp.Spec.RouteSpec.Hostname, + Name: resourceName, Labels: map[string]string{ CappResourceKey: capp.Name, cappNamespaceKey: capp.Namespace, @@ -100,7 +89,7 @@ func (r ARecordSetManager) prepareResource(capp cappv1alpha1.Capp) (dnsv1alpha1. }, Spec: dnsv1alpha1.ARecordSetSpec{ ForProvider: dnsv1alpha1.ARecordSetParameters{ - Name: &name, + Name: &recordName, Zone: &zone, Addresses: addresses, }, diff --git a/internal/kinds/capp/resourcemanagers/certificate.go b/internal/kinds/capp/resourcemanagers/certificate.go index 38bd766d..e248f77d 100644 --- a/internal/kinds/capp/resourcemanagers/certificate.go +++ b/internal/kinds/capp/resourcemanagers/certificate.go @@ -6,6 +6,7 @@ import ( certv1alpha1 "github.com/dana-team/certificate-operator/api/v1alpha1" cappv1alpha1 "github.com/dana-team/container-app-operator/api/v1alpha1" + "github.com/dana-team/container-app-operator/internal/kinds/capp/utils" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -35,11 +36,17 @@ type CertificateManager struct { } // prepareResource prepares a Certificate resource based on the provided Capp. -func (c CertificateManager) prepareResource(capp cappv1alpha1.Capp) certv1alpha1.Certificate { - return certv1alpha1.Certificate{ +func (c CertificateManager) prepareResource(capp cappv1alpha1.Capp) (certv1alpha1.Certificate, error) { + zone, err := utils.GetZoneFromConfig(c.Ctx, c.K8sclient) + if err != nil { + return certv1alpha1.Certificate{}, err + } + resourceName := utils.GenerateResourceName(capp.Spec.RouteSpec.Hostname, zone) + + certificate := certv1alpha1.Certificate{ TypeMeta: metav1.TypeMeta{}, ObjectMeta: metav1.ObjectMeta{ - Name: capp.Spec.RouteSpec.Hostname, + Name: resourceName, Namespace: capp.Namespace, Labels: map[string]string{ CappResourceKey: capp.Name, @@ -48,10 +55,10 @@ func (c CertificateManager) prepareResource(capp cappv1alpha1.Capp) certv1alpha1 Spec: certv1alpha1.CertificateSpec{ CertificateData: certv1alpha1.CertificateData{ Subject: certv1alpha1.Subject{ - CommonName: capp.Spec.RouteSpec.Hostname, + CommonName: resourceName, }, San: certv1alpha1.San{ - DNS: []string{capp.Spec.RouteSpec.Hostname}, + DNS: []string{resourceName}, }, Form: certificateForm, }, @@ -61,6 +68,8 @@ func (c CertificateManager) prepareResource(capp cappv1alpha1.Capp) certv1alpha1 }, }, } + + return certificate, nil } // CleanUp attempts to delete the associated Certificate for a given Capp resource. @@ -96,7 +105,11 @@ func (c CertificateManager) Manage(capp cappv1alpha1.Capp) error { // create creates a Certificate resource. func (c CertificateManager) create(capp cappv1alpha1.Capp) error { - certificateFromCapp := c.prepareResource(capp) + certificateFromCapp, err := c.prepareResource(capp) + if err != nil { + return fmt.Errorf("failed to prepare Certificate: %w", err) + } + certificate := certv1alpha1.Certificate{} resourceManager := rclient.ResourceManagerClient{Ctx: c.Ctx, K8sclient: c.K8sclient, Log: c.Log} diff --git a/internal/kinds/capp/resourcemanagers/domainmapping.go b/internal/kinds/capp/resourcemanagers/domainmapping.go index 4f6492ee..61cfb278 100644 --- a/internal/kinds/capp/resourcemanagers/domainmapping.go +++ b/internal/kinds/capp/resourcemanagers/domainmapping.go @@ -5,6 +5,8 @@ import ( "fmt" "reflect" + "github.com/dana-team/container-app-operator/internal/kinds/capp/utils" + rclient "github.com/dana-team/container-app-operator/internal/kinds/capp/resourceclient" cappv1alpha1 "github.com/dana-team/container-app-operator/api/v1alpha1" @@ -39,11 +41,17 @@ type KnativeDomainMappingManager struct { } // PrepareKnativeDomainMapping creates a new DomainMapping for a Knative service. -func (k KnativeDomainMappingManager) prepareResource(capp cappv1alpha1.Capp) knativev1beta1.DomainMapping { +func (k KnativeDomainMappingManager) prepareResource(capp cappv1alpha1.Capp) (knativev1beta1.DomainMapping, error) { + zone, err := utils.GetZoneFromConfig(k.Ctx, k.K8sclient) + if err != nil { + return knativev1beta1.DomainMapping{}, err + } + resourceName := utils.GenerateResourceName(capp.Spec.RouteSpec.Hostname, zone) + knativeDomainMapping := &knativev1beta1.DomainMapping{ TypeMeta: metav1.TypeMeta{}, ObjectMeta: metav1.ObjectMeta{ - Name: capp.Spec.RouteSpec.Hostname, + Name: resourceName, Namespace: capp.Namespace, Labels: map[string]string{ CappResourceKey: capp.Name, @@ -64,7 +72,7 @@ func (k KnativeDomainMappingManager) prepareResource(capp cappv1alpha1.Capp) kna } } - return *knativeDomainMapping + return *knativeDomainMapping, nil } // CleanUp attempts to delete the associated DomainMapping for a given Capp resource. @@ -100,7 +108,11 @@ func (k KnativeDomainMappingManager) Manage(capp cappv1alpha1.Capp) error { // createOrUpdate creates or updates a DomainMapping resource. func (k KnativeDomainMappingManager) createOrUpdate(capp cappv1alpha1.Capp) error { - domainMappingFromCapp := k.prepareResource(capp) + domainMappingFromCapp, err := k.prepareResource(capp) + if err != nil { + return fmt.Errorf("failed to prepare DomainMapping: %w", err) + } + domainMapping := knativev1beta1.DomainMapping{} resourceManager := rclient.ResourceManagerClient{Ctx: k.Ctx, K8sclient: k.K8sclient, Log: k.Log} diff --git a/internal/kinds/capp/resourcemanagers/ksvc.go b/internal/kinds/capp/resourcemanagers/ksvc.go index 02041aa5..a5b0ff60 100644 --- a/internal/kinds/capp/resourcemanagers/ksvc.go +++ b/internal/kinds/capp/resourcemanagers/ksvc.go @@ -25,8 +25,7 @@ const ( danaAnnotationsPrefix = "rcs.dana.io" cappDisabledState = "disabled" cappEnabledState = "enabled" - DefaultAutoScaleCM = "autoscale-defaults" - CappNS = "capp-operator-system" + defaultAutoScaleCM = "autoscale-defaults" KnativeServing = "knativeServing" eventCappKnativeServiceCreationFailed = "KnativeServiceCreationFailed" eventCappKnativeServiceCreated = "KnativeServiceCreated" @@ -77,8 +76,8 @@ func (k KnativeServiceManager) prepareResource(capp cappv1alpha1.Capp, ctx conte knativeService.Spec.Template.Spec.Volumes = append(knativeService.Spec.Template.Spec.Volumes, volumes...) defaultCM := corev1.ConfigMap{} - if err := k.K8sclient.Get(k.Ctx, types.NamespacedName{Namespace: CappNS, Name: DefaultAutoScaleCM}, &defaultCM); err != nil { - k.Log.Error(err, fmt.Sprintf("could not fetch configMap: %q", CappNS)) + if err := k.K8sclient.Get(k.Ctx, types.NamespacedName{Namespace: utils.CappNS, Name: defaultAutoScaleCM}, &defaultCM); err != nil { + k.Log.Error(err, fmt.Sprintf("could not fetch configMap from namespace %q", utils.CappNS)) } knativeService.Spec.Template.ObjectMeta.Annotations = utils.MergeMaps(knativeServiceAnnotations, autoscale.SetAutoScaler(capp, defaultCM.Data)) diff --git a/internal/kinds/capp/status/route.go b/internal/kinds/capp/status/route.go index a7bd95b1..d73bcedc 100644 --- a/internal/kinds/capp/status/route.go +++ b/internal/kinds/capp/status/route.go @@ -4,6 +4,7 @@ import ( "context" rmanagers "github.com/dana-team/container-app-operator/internal/kinds/capp/resourcemanagers" + "github.com/dana-team/container-app-operator/internal/kinds/capp/utils" certv1alpha1 "github.com/dana-team/certificate-operator/api/v1alpha1" cappv1alpha1 "github.com/dana-team/container-app-operator/api/v1alpha1" @@ -18,17 +19,22 @@ import ( func buildRouteStatus(ctx context.Context, kubeClient client.Client, capp cappv1alpha1.Capp, isRequired map[string]bool) (cappv1alpha1.RouteStatus, error) { routeStatus := cappv1alpha1.RouteStatus{} - domainMappingStatus, err := buildDomainMappingStatus(ctx, kubeClient, capp, isRequired[rmanagers.DomainMapping]) + zone, err := utils.GetZoneFromConfig(ctx, kubeClient) if err != nil { return routeStatus, err } - aRecordSetStatus, err := buildARecordSetStatus(ctx, kubeClient, capp, isRequired[rmanagers.ARecordSet]) + domainMappingStatus, err := buildDomainMappingStatus(ctx, kubeClient, capp, isRequired[rmanagers.DomainMapping], zone) if err != nil { return routeStatus, err } - certificateStatus, err := buildCertificateStatus(ctx, kubeClient, capp, isRequired[rmanagers.Certificate]) + aRecordSetStatus, err := buildARecordSetStatus(ctx, kubeClient, capp, isRequired[rmanagers.ARecordSet], zone) + if err != nil { + return routeStatus, err + } + + certificateStatus, err := buildCertificateStatus(ctx, kubeClient, capp, isRequired[rmanagers.Certificate], zone) if err != nil { return routeStatus, err } @@ -42,13 +48,13 @@ func buildRouteStatus(ctx context.Context, kubeClient client.Client, capp cappv1 // buildDomainMappingStatus partly constructs the Route Status of the Capp object in accordance to the // status of the corresponding DomainMapping object. -func buildDomainMappingStatus(ctx context.Context, kubeClient client.Client, capp cappv1alpha1.Capp, isRequired bool) (knativev1beta1.DomainMappingStatus, error) { +func buildDomainMappingStatus(ctx context.Context, kubeClient client.Client, capp cappv1alpha1.Capp, isRequired bool, zone string) (knativev1beta1.DomainMappingStatus, error) { if !isRequired { return knativev1beta1.DomainMappingStatus{}, nil } domainMapping := &knativev1beta1.DomainMapping{} - domainMappingName := capp.Spec.RouteSpec.Hostname + domainMappingName := utils.GenerateResourceName(capp.Spec.RouteSpec.Hostname, zone) if err := kubeClient.Get(ctx, types.NamespacedName{Namespace: capp.Namespace, Name: domainMappingName}, domainMapping); err != nil { return knativev1beta1.DomainMappingStatus{}, err } @@ -58,13 +64,13 @@ func buildDomainMappingStatus(ctx context.Context, kubeClient client.Client, cap // buildCertificateStatus partly constructs the Route Status of the Capp object in accordance to the // status of the corresponding Certificate object. -func buildCertificateStatus(ctx context.Context, kubeClient client.Client, capp cappv1alpha1.Capp, isRequired bool) (certv1alpha1.CertificateStatus, error) { +func buildCertificateStatus(ctx context.Context, kubeClient client.Client, capp cappv1alpha1.Capp, isRequired bool, zone string) (certv1alpha1.CertificateStatus, error) { if !isRequired { return certv1alpha1.CertificateStatus{}, nil } certificate := &certv1alpha1.Certificate{} - certificateName := capp.Spec.RouteSpec.Hostname + certificateName := utils.GenerateResourceName(capp.Spec.RouteSpec.Hostname, zone) if err := kubeClient.Get(ctx, types.NamespacedName{Namespace: capp.Namespace, Name: certificateName}, certificate); err != nil { return certv1alpha1.CertificateStatus{}, err } @@ -74,13 +80,14 @@ func buildCertificateStatus(ctx context.Context, kubeClient client.Client, capp // buildARecordSetStatus partly constructs the Route Status of the Capp object in accordance to the // status of the corresponding ARecordSet object. -func buildARecordSetStatus(ctx context.Context, kubeClient client.Client, capp cappv1alpha1.Capp, isRequired bool) (dnsv1alpha1.ARecordSetStatus, error) { +func buildARecordSetStatus(ctx context.Context, kubeClient client.Client, capp cappv1alpha1.Capp, isRequired bool, zone string) (dnsv1alpha1.ARecordSetStatus, error) { if !isRequired { return dnsv1alpha1.ARecordSetStatus{}, nil } aRecordSet := &dnsv1alpha1.ARecordSet{} - if err := kubeClient.Get(ctx, types.NamespacedName{Name: capp.Spec.RouteSpec.Hostname}, aRecordSet); err != nil { + aRecordSetName := utils.GenerateResourceName(capp.Spec.RouteSpec.Hostname, zone) + if err := kubeClient.Get(ctx, types.NamespacedName{Name: aRecordSetName}, aRecordSet); err != nil { return dnsv1alpha1.ARecordSetStatus{}, err } diff --git a/internal/kinds/capp/utils/route.go b/internal/kinds/capp/utils/route.go new file mode 100644 index 00000000..d79bb89f --- /dev/null +++ b/internal/kinds/capp/utils/route.go @@ -0,0 +1,64 @@ +package utils + +import ( + "context" + "fmt" + "strings" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +const ( + zoneCM = "dns-zone" + zoneKey = "zone" + placeholderZone = "capp.com." + dot = "." +) + +// GetZoneFromConfig returns the zone to be used for the record from a ConfigMap. +func GetZoneFromConfig(ctx context.Context, k8sClient client.Client) (string, error) { + var ok bool + + routeCM := corev1.ConfigMap{} + if err := k8sClient.Get(ctx, types.NamespacedName{Namespace: CappNS, Name: zoneCM}, &routeCM); err != nil { + return "", fmt.Errorf("could not fetch configMap %q from namespace %q: %w", zoneCM, CappNS, err) + } + + zone := placeholderZone + if len(routeCM.Data) > 0 { + zone, ok = routeCM.Data[zoneKey] + if !ok { + return zone, fmt.Errorf("%q key is not set in ConfigMap %q", zoneKey, zoneCM) + } else if zone == "" { + return zone, fmt.Errorf("%q is empty in ConfigMap %q", zoneKey, zoneCM) + } else if !strings.HasSuffix(zone, dot) { + return zone, fmt.Errorf("%q value must end with a %q in ConfigMap %q", zoneKey, dot, zoneCM) + } + } + + return zone, nil +} + +// GenerateResourceName generates the hostname based on the provided suffix and a dot(".") trailing character. +// If the hostname does not already end with the suffix (minus the trailing dot), it appends the suffix to the hostname. +func GenerateResourceName(hostname, suffix string) string { + suffixWithoutTrailingChar := suffix[:len(suffix)-len(dot)] + if !strings.HasSuffix(hostname, suffixWithoutTrailingChar) { + return hostname + dot + suffixWithoutTrailingChar + } + + return hostname +} + +// GenerateRecordName generates the hostname based on the provided suffix and a dot(".") trailing character. +// It returns the original hostname with the suffix removed if it was present, otherwise the original hostname. +func GenerateRecordName(hostname, suffix string) string { + suffixWithoutTrailingChar := suffix[:len(suffix)-len(dot)] + if !strings.HasSuffix(hostname, suffixWithoutTrailingChar) { + return hostname + } + + return hostname[:len(hostname)-len(suffix)] +} diff --git a/internal/kinds/capp/utils/utils.go b/internal/kinds/capp/utils/utils.go index e83d9d87..a7aa6d95 100644 --- a/internal/kinds/capp/utils/utils.go +++ b/internal/kinds/capp/utils/utils.go @@ -8,6 +8,8 @@ import ( "k8s.io/client-go/rest" ) +const CappNS = "capp-operator-system" + // IsOnOpenshift returns true if the cluster has the openshift config group func IsOnOpenshift(config *rest.Config) (bool, error) { dc, err := discovery.NewDiscoveryClientForConfig(config) diff --git a/test/e2e_tests/arecordset_e2e_test.go b/test/e2e_tests/arecordset_e2e_test.go index b1e0bfb9..464ede2b 100644 --- a/test/e2e_tests/arecordset_e2e_test.go +++ b/test/e2e_tests/arecordset_e2e_test.go @@ -15,7 +15,8 @@ var _ = Describe("Validate ARecordSet functionality", func() { createdCapp, _ := utilst.CreateCappWithHTTPHostname(k8sClient) By("Checking if the ARecordSet was created successfully") - aRecordSetObject := mock.CreateARecordSetObject(createdCapp.Spec.RouteSpec.Hostname) + aRecordSetName := utilst.GenerateResourceName(createdCapp.Spec.RouteSpec.Hostname, mock.ZoneValue) + aRecordSetObject := mock.CreateARecordSetObject(aRecordSetName) Eventually(func() bool { return utilst.DoesResourceExist(k8sClient, aRecordSetObject) }, testconsts.Timeout, testconsts.Interval).Should(BeTrue(), "Should find a resource.") @@ -27,8 +28,9 @@ var _ = Describe("Validate ARecordSet functionality", func() { utilst.UpdateCapp(k8sClient, toBeUpdatedCapp) updatedARecordSet := aRecordSetObject + updatedARecordSetName := utilst.GenerateResourceName(updatedRouteHostname, mock.ZoneValue) Eventually(func() *string { - updatedARecordSet = utilst.GetARecordSet(k8sClient, updatedRouteHostname) + updatedARecordSet = utilst.GetARecordSet(k8sClient, updatedARecordSetName) return updatedARecordSet.Spec.ForProvider.Name }, testconsts.Timeout, testconsts.Interval).Should(Equal(&updatedRouteHostname)) @@ -49,7 +51,8 @@ var _ = Describe("Validate ARecordSet functionality", func() { createdCapp, _ := utilst.CreateCappWithHTTPHostname(k8sClient) By("Checking if the ARecordSet was created successfully") - aRecordSetObject := mock.CreateARecordSetObject(createdCapp.Spec.RouteSpec.Hostname) + aRecordSetName := utilst.GenerateResourceName(createdCapp.Spec.RouteSpec.Hostname, mock.ZoneValue) + aRecordSetObject := mock.CreateARecordSetObject(aRecordSetName) Eventually(func() bool { return utilst.DoesResourceExist(k8sClient, aRecordSetObject) }, testconsts.Timeout, testconsts.Interval).Should(BeTrue(), "Should find a resource.") diff --git a/test/e2e_tests/certificate_e2e_test.go b/test/e2e_tests/certificate_e2e_test.go index 4bbc7169..98243956 100644 --- a/test/e2e_tests/certificate_e2e_test.go +++ b/test/e2e_tests/certificate_e2e_test.go @@ -15,14 +15,15 @@ var _ = Describe("Validate Certificate functionality", func() { createdCapp, routeHostname, _ := utilst.CreateHTTPSCapp(k8sClient) By("Checking if the Certificate was created successfully") - certificateObject := mock.CreateCertificateObject(routeHostname) + certificateName := utilst.GenerateResourceName(routeHostname, mock.ZoneValue) + certificateObject := mock.CreateCertificateObject(certificateName) Eventually(func() bool { return utilst.DoesResourceExist(k8sClient, certificateObject) }, testconsts.Timeout, testconsts.Interval).Should(BeTrue(), "Should find a resource.") By("Updating the Capp Route hostname and checking the status") toBeUpdatedCapp := utilst.GetCapp(k8sClient, createdCapp.Name, createdCapp.Namespace) - updatedRouteHostname := utilst.GenerateRouteHostname() + updatedRouteHostname := utilst.GenerateResourceName(utilst.GenerateRouteHostname(), mock.ZoneValue) toBeUpdatedCapp.Spec.RouteSpec.Hostname = updatedRouteHostname utilst.UpdateCapp(k8sClient, toBeUpdatedCapp) @@ -57,10 +58,11 @@ var _ = Describe("Validate Certificate functionality", func() { _, routeHostname := utilst.CreateCappWithHTTPHostname(k8sClient) By("Checking if the Certificate was not created") - certificateObject := mock.CreateCertificateObject(routeHostname) + certificateName := utilst.GenerateResourceName(routeHostname, mock.ZoneValue) + certificateObject := mock.CreateCertificateObject(certificateName) Consistently(func() bool { return utilst.DoesResourceExist(k8sClient, certificateObject) - }, testconsts.Timeout, testconsts.Interval).Should(BeFalse(), "Should not find a resource.") + }, testconsts.DefaultConsistently, testconsts.Interval).Should(BeFalse(), "Should not find a resource.") }) It("Should cleanup Certificate when no longer required (tls)", func() { @@ -68,7 +70,8 @@ var _ = Describe("Validate Certificate functionality", func() { createdCapp, routeHostname, _ := utilst.CreateHTTPSCapp(k8sClient) By("Checking if the Certificate was created successfully") - certificateObject := mock.CreateCertificateObject(routeHostname) + certificateName := utilst.GenerateResourceName(routeHostname, mock.ZoneValue) + certificateObject := mock.CreateCertificateObject(certificateName) Eventually(func() bool { return utilst.DoesResourceExist(k8sClient, certificateObject) }, testconsts.Timeout, testconsts.Interval).Should(BeTrue(), "Should find a resource.") @@ -90,7 +93,8 @@ var _ = Describe("Validate Certificate functionality", func() { createdCapp, routeHostname, _ := utilst.CreateHTTPSCapp(k8sClient) By("Checking if the Certificate was created successfully") - certificateObject := mock.CreateCertificateObject(routeHostname) + certificateName := utilst.GenerateResourceName(routeHostname, mock.ZoneValue) + certificateObject := mock.CreateCertificateObject(certificateName) Eventually(func() bool { return utilst.DoesResourceExist(k8sClient, certificateObject) }, testconsts.Timeout, testconsts.Interval).Should(BeTrue(), "Should find a resource.") diff --git a/test/e2e_tests/domain_mapping_e2e_test.go b/test/e2e_tests/domain_mapping_e2e_test.go index 24ecfa53..5d7b71e0 100644 --- a/test/e2e_tests/domain_mapping_e2e_test.go +++ b/test/e2e_tests/domain_mapping_e2e_test.go @@ -16,7 +16,8 @@ var _ = Describe("Validate DomainMapping functionality", func() { createdCapp, routeHostname := utilst.CreateCappWithHTTPHostname(k8sClient) By("Checking if the domainMapping was created successfully") - domainMappingObject := mock.CreateDomainMappingObject(routeHostname) + domainMappingName := utilst.GenerateResourceName(routeHostname, mock.ZoneValue) + domainMappingObject := mock.CreateDomainMappingObject(domainMappingName) Eventually(func() bool { return utilst.DoesResourceExist(k8sClient, domainMappingObject) }, testconsts.Timeout, testconsts.Interval).Should(BeTrue(), "Should find a resource.") @@ -28,11 +29,11 @@ var _ = Describe("Validate DomainMapping functionality", func() { return "" } return capp.Status.RouteStatus.DomainMappingObjectStatus.URL.Host - }, testconsts.Timeout, testconsts.Interval).Should(Equal(routeHostname), "Should update Route Status of Capp") + }, testconsts.Timeout, testconsts.Interval).Should(Equal(domainMappingName), "Should update Route Status of Capp") By("Updating the Capp Route hostname and checking the status") toBeUpdatedCapp := utilst.GetCapp(k8sClient, createdCapp.Name, createdCapp.Namespace) - updatedRouteHostname := utilst.GenerateRouteHostname() + updatedRouteHostname := utilst.GenerateResourceName(utilst.GenerateRouteHostname(), mock.ZoneValue) toBeUpdatedCapp.Spec.RouteSpec.Hostname = updatedRouteHostname utilst.UpdateCapp(k8sClient, toBeUpdatedCapp) @@ -69,8 +70,9 @@ var _ = Describe("Validate DomainMapping functionality", func() { utilst.CreateSecret(k8sClient, secretObject) By("Checking if the secret reference exists at the domainMapping") + domainMappingName := utilst.GenerateResourceName(routeHostname, mock.ZoneValue) Eventually(func() string { - domainMapping := utilst.GetDomainMapping(k8sClient, routeHostname, createdCapp.Namespace) + domainMapping := utilst.GetDomainMapping(k8sClient, domainMappingName, createdCapp.Namespace) return domainMapping.Spec.TLS.SecretName }, testconsts.Timeout, testconsts.Interval).Should(Equal(secretName)) }) @@ -79,18 +81,19 @@ var _ = Describe("Validate DomainMapping functionality", func() { By("Creating a capp with a route") createdCapp, routeHostname := utilst.CreateCappWithHTTPHostname(k8sClient) + domainMappingName := utilst.GenerateResourceName(routeHostname, mock.ZoneValue) By("Checking if the RouteStatus of the Capp was updated successfully") Eventually(func() string { capp := utilst.GetCapp(k8sClient, createdCapp.Name, createdCapp.Namespace) return capp.Status.RouteStatus.DomainMappingObjectStatus.URL.Host - }, testconsts.Timeout, testconsts.Interval).Should(Equal(routeHostname), "Should update Route Status of Capp") + }, testconsts.Timeout, testconsts.Interval).Should(Equal(domainMappingName), "Should update Route Status of Capp") By("Removing the Route from the Capp and check the status and resource clean up") toBeUpdatedCapp := utilst.GetCapp(k8sClient, createdCapp.Name, createdCapp.Namespace) toBeUpdatedCapp.Spec.RouteSpec = cappv1alpha1.RouteSpec{} utilst.UpdateCapp(k8sClient, toBeUpdatedCapp) - domainMappingObject := mock.CreateDomainMappingObject(routeHostname) + domainMappingObject := mock.CreateDomainMappingObject(domainMappingName) Eventually(func() bool { return utilst.DoesResourceExist(k8sClient, domainMappingObject) }, testconsts.Timeout, testconsts.Interval).ShouldNot(BeTrue(), "Should not find a resource.") diff --git a/test/e2e_tests/knative_e2e_test.go b/test/e2e_tests/knative_e2e_test.go index 60263fb3..02a690c5 100644 --- a/test/e2e_tests/knative_e2e_test.go +++ b/test/e2e_tests/knative_e2e_test.go @@ -307,7 +307,7 @@ var _ = Describe("Validate knative functionality", func() { Consistently(func() string { ksvc := utilst.GetKSVC(k8sClient, assertionCapp.Name, assertionCapp.Namespace) return ksvc.Spec.ConfigurationSpec.Template.Labels[testconsts.CappResourceKey] - }, testconsts.Timeout, testconsts.Interval).ShouldNot(Equal("test")) + }, testconsts.DefaultConsistently, testconsts.Interval).ShouldNot(Equal("test")) Eventually(func() string { ksvc := utilst.GetKSVC(k8sClient, assertionCapp.Name, assertionCapp.Namespace) diff --git a/test/e2e_tests/mocks/base.go b/test/e2e_tests/mocks/base.go index 7999cfd6..68b7c51b 100644 --- a/test/e2e_tests/mocks/base.go +++ b/test/e2e_tests/mocks/base.go @@ -15,6 +15,9 @@ const ( SecretValue = "YmFyCg==" ControllerNS = "capp-operator-system" AutoScaleCM = "autoscale-defaults" + ZoneCM = "dns-zone" + ZoneKey = "zone" + ZoneValue = "test-zone.com." ) func CreateBaseCapp() *cappv1alpha1.Capp { diff --git a/test/e2e_tests/mocks/certificate.go b/test/e2e_tests/mocks/certificate.go deleted file mode 100644 index 6c08e238..00000000 --- a/test/e2e_tests/mocks/certificate.go +++ /dev/null @@ -1,16 +0,0 @@ -package mocks - -import ( - certv1alpha1 "github.com/dana-team/certificate-operator/api/v1alpha1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - -// CreateCertificateObject returns an empty DomainMapping object. -func CreateCertificateObject(name string) *certv1alpha1.Certificate { - return &certv1alpha1.Certificate{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: NSName, - }, - } -} diff --git a/test/e2e_tests/mocks/dnsendpoint.go b/test/e2e_tests/mocks/dnsendpoint.go deleted file mode 100644 index 78e7c3a3..00000000 --- a/test/e2e_tests/mocks/dnsendpoint.go +++ /dev/null @@ -1,15 +0,0 @@ -package mocks - -import ( - dnsv1alpha1 "github.com/dana-team/provider-dns/apis/recordset/v1alpha1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - -// CreateARecordSetObject returns an empty ARecordSet object. -func CreateARecordSetObject(name string) *dnsv1alpha1.ARecordSet { - return &dnsv1alpha1.ARecordSet{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - }, - } -} diff --git a/test/e2e_tests/mocks/domain_mapping.go b/test/e2e_tests/mocks/domain_mapping.go deleted file mode 100644 index 53e5fe75..00000000 --- a/test/e2e_tests/mocks/domain_mapping.go +++ /dev/null @@ -1,16 +0,0 @@ -package mocks - -import ( - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - knativev1beta1 "knative.dev/serving/pkg/apis/serving/v1beta1" -) - -// CreateDomainMappingObject returns an empty DomainMapping object. -func CreateDomainMappingObject(name string) *knativev1beta1.DomainMapping { - return &knativev1beta1.DomainMapping{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: NSName, - }, - } -} diff --git a/test/e2e_tests/mocks/route.go b/test/e2e_tests/mocks/route.go new file mode 100644 index 00000000..94be20fd --- /dev/null +++ b/test/e2e_tests/mocks/route.go @@ -0,0 +1,37 @@ +package mocks + +import ( + certv1alpha1 "github.com/dana-team/certificate-operator/api/v1alpha1" + dnsv1alpha1 "github.com/dana-team/provider-dns/apis/recordset/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + knativev1beta1 "knative.dev/serving/pkg/apis/serving/v1beta1" +) + +// CreateDomainMappingObject returns an empty DomainMapping object. +func CreateDomainMappingObject(name string) *knativev1beta1.DomainMapping { + return &knativev1beta1.DomainMapping{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: NSName, + }, + } +} + +// CreateCertificateObject returns an empty DomainMapping object. +func CreateCertificateObject(name string) *certv1alpha1.Certificate { + return &certv1alpha1.Certificate{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: NSName, + }, + } +} + +// CreateARecordSetObject returns an empty ARecordSet object. +func CreateARecordSetObject(name string) *dnsv1alpha1.ARecordSet { + return &dnsv1alpha1.ARecordSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + } +} diff --git a/test/e2e_tests/suite_test.go b/test/e2e_tests/suite_test.go index 3c0d52ba..0350a625 100644 --- a/test/e2e_tests/suite_test.go +++ b/test/e2e_tests/suite_test.go @@ -29,6 +29,7 @@ var _ = SynchronizedBeforeSuite(func() { createE2ETestNamespace() initE2ETestAutoScaleConfigMap() createE2ETestAutoScaleConfigMap() + createE2ETestZoneConfigMap() }, func() { initClient() initE2ETestAutoScaleConfigMap() @@ -72,12 +73,22 @@ func initE2ETestAutoScaleConfigMap() { targetAutoScale = map[string]string{"rps": rps, "cpu": cpu, "memory": memory, "concurrency": concurrency} } -// createE2ETestAutoScaleConfigMap creates an Auto Scale ConfigMap for the e2e tests. +// createE2ETestAutoScaleConfigMap creates an Autoscale ConfigMap for the e2e tests. func createE2ETestAutoScaleConfigMap() { autoScaleConfigMap := mock.CreateConfigMapObject(mock.ControllerNS, mock.AutoScaleCM, targetAutoScale) utilst.CreateConfigMap(k8sClient, autoScaleConfigMap) } +// createE2ETestZoneConfigMap creates a Zone ConfigMap for the e2e tests. +func createE2ETestZoneConfigMap() { + zone := map[string]string{ + mock.ZoneKey: mock.ZoneValue, + } + + zoneConfigMap := mock.CreateConfigMapObject(mock.ControllerNS, mock.ZoneCM, zone) + utilst.CreateConfigMap(k8sClient, zoneConfigMap) +} + // cleanUp make sure the test environment is clean. func cleanUp() { namespace := &corev1.Namespace{ @@ -85,16 +96,31 @@ func cleanUp() { Name: mock.NSName, }, } + configMap := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{ Name: mock.AutoScaleCM, Namespace: mock.ControllerNS, }} + if utilst.DoesResourceExist(k8sClient, configMap) { Expect(k8sClient.Delete(context.Background(), configMap)).To(Succeed()) Eventually(func() error { return k8sClient.Get(context.Background(), client.ObjectKey{Name: mock.AutoScaleCM, Namespace: mock.ControllerNS}, configMap) }, testconsts.Timeout, testconsts.Interval).Should(HaveOccurred(), "The autoscale configMap should be deleted") } + + configMap = &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{ + Name: mock.ZoneCM, + Namespace: mock.ControllerNS, + }} + + if utilst.DoesResourceExist(k8sClient, configMap) { + Expect(k8sClient.Delete(context.Background(), configMap)).To(Succeed()) + Eventually(func() error { + return k8sClient.Get(context.Background(), client.ObjectKey{Name: mock.ZoneCM, Namespace: mock.ControllerNS}, configMap) + }, testconsts.Timeout, testconsts.Interval).Should(HaveOccurred(), "The autoscale configMap should be deleted") + } + if utilst.DoesResourceExist(k8sClient, namespace) { Expect(k8sClient.Delete(context.Background(), namespace)).To(Succeed()) Eventually(func() error { diff --git a/test/e2e_tests/testconsts/testconsts.go b/test/e2e_tests/testconsts/testconsts.go index d81b49f7..aa966549 100644 --- a/test/e2e_tests/testconsts/testconsts.go +++ b/test/e2e_tests/testconsts/testconsts.go @@ -3,9 +3,10 @@ package testconsts import "time" const ( - Timeout = 300 * time.Second - Interval = 2 * time.Second - DefaultEventually = 2 * time.Second + Timeout = 300 * time.Second + Interval = 2 * time.Second + DefaultEventually = 2 * time.Second + DefaultConsistently = 30 * time.Second ) const ( diff --git a/test/e2e_tests/utils/resources_adpater.go b/test/e2e_tests/utils/resources_adpater.go index 81d729cb..4b771153 100644 --- a/test/e2e_tests/utils/resources_adpater.go +++ b/test/e2e_tests/utils/resources_adpater.go @@ -68,7 +68,7 @@ func CreateSecret(k8sClient client.Client, secret *corev1.Secret) { // CreateConfigMap creates a new configMap. func CreateConfigMap(k8sClient client.Client, configMap *corev1.ConfigMap) { - Expect(k8sClient.Create(context.Background(), configMap)).To(Succeed()) + Expect(k8sClient.Create(context.Background(), configMap)).To(SatisfyAny(BeNil(), WithTransform(errors.IsAlreadyExists, BeTrue()))) } // GenerateRouteHostname generates a new route hostname by calling diff --git a/test/e2e_tests/utils/route_adapter.go b/test/e2e_tests/utils/route_adapter.go index 7075e3ef..378f8a53 100644 --- a/test/e2e_tests/utils/route_adapter.go +++ b/test/e2e_tests/utils/route_adapter.go @@ -1,6 +1,8 @@ package utils import ( + "strings" + certv1alpha1 "github.com/dana-team/certificate-operator/api/v1alpha1" cappv1alpha1 "github.com/dana-team/container-app-operator/api/v1alpha1" mock "github.com/dana-team/container-app-operator/test/e2e_tests/mocks" @@ -9,6 +11,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) +const dot = "." + // CreateCappWithHTTPHostname creates a Capp with a Hostname. func CreateCappWithHTTPHostname(k8sClient client.Client) (*cappv1alpha1.Capp, string) { httpsCapp := mock.CreateBaseCapp() @@ -52,3 +56,15 @@ func GetCertificate(k8sClient client.Client, name string, namespace string) *cer GetResource(k8sClient, certificate, name, namespace) return certificate } + +// GenerateResourceName generates the hostname based on the provided suffix and a dot (".") trailing character. +// It returns the adjusted hostname, where the suffix (minus the trailing character) is added if not already present. +func GenerateResourceName(hostname, suffix string) string { + suffixWithoutTrailingChar := suffix[:len(suffix)-len(dot)] + + if !strings.HasSuffix(hostname, suffixWithoutTrailingChar) { + return hostname + dot + suffixWithoutTrailingChar + } + + return hostname +}