Runtime Agent fails to apply Application CR when application name is not normalized

[NickyMateev]

Overview

Description

When runtime agent regularly fetches the applications for the runtime from Compass it tries to create Application CRs for each.
The problem is that if an application is retrieved from Compass which is not with a normalized DNS name, the runtime agent fails to create the Application CR.

Expected result

Currently the Services part of the same Application CR are sanitized/normalized so that the runtime agent is sure that the name is DNS compliant so that later on the necessary Service Catalog ServiceClasses and ServicePlans CRs can be created.

Reference 1:

kyma/components/compass-runtime-agent/internal/kyma/applications/converter.go

Line 54 in ef34835

Services: c.toServices(application.Name, application.ProviderDisplayName, application.APIPackages),

Reference 2:

kyma/components/compass-runtime-agent/internal/kyma/applications/converter.go

Line 80 in ef34835

Name: createServiceName(apiPackage.Name, apiPackage.ID),

Reference 3:

kyma/components/compass-runtime-agent/internal/kyma/applications/util.go

Line 19 in ef34835

// createServiceName creates the OSB Service Name for given Application Service.

I would expect that the runtime agent normalizes the application name as well so that the Application CR can be created without any issues. But this doesn't happen as we can see that the value is used directly as retrieved from Compass:

kyma/components/compass-runtime-agent/internal/kyma/applications/converter.go

Line 48 in ef34835

Name: application.Name,

I would expect that runtime agent ensures all application names are normalized (like it does for Packages into Services) rather than expect Compass to provide DNS compliant application names.

Actual result

The Application "-123wordpress" is invalid: metadata.name: Invalid value: "-123wordpress": a DNS-1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and │
│  must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')

Steps to reproduce

Quick way to reproduce: Create Application CR with non-DNS compliant name.

Long way to reproduce:

1. Pair Kyma Runtime with Compass
2. Register application with non-DNS compliant name
3. Place application in scenario with runtime
4. Observe runtime agent logs -> should find issues when trying to create the Application CR

[NickyMateev]

Proposal

After further discussion with @crabtree and @PK85 here's the following proposal we jointly formed together in order to achieve the desired outcome:

1. Runtime Agent should start normalizing the Application CR resource name:

   kyma/components/compass-runtime-agent/internal/kyma/applications/converter.go

   Line 48 in ef34835

   Name: application.Name,

   • The following algorithm for normalization should be used (it's important to use this same algorithm for backwards compatibility reasons with Compass and existing Kyma runtimes): https://github.com/kyma-incubator/compass/blob/3fe1796bc7a273d08dff10c53860b9793ff40a23/components/director/pkg/normalizer/normalizer.go#L31
   • It's possible for different unnormalized application names to produce the same normalized name, hence Runtime Agent should append some kind of unique suffix so as to eliminate potential naming conflicts with existing Application CR names
   • For smooth transition and migration between the Runtime Agent and Compass we currently have the following setup: all runtimes in Compass are labeled with isNormalized=true by default. When that's the case Compass makes sure to normalize all Application names when the Runtime Agent fetches its applications (applicatoinsForRuntime query). Runtime Agent should apply its custom normalization logic for Application CR resource names only if runtime is labeled with isNormalized=false. So this way once all Kyma runtimes are updated with the new normalization logic we could flip the switch centrally in Compass to isNormalized=false - Compass will stop normalizing (the names from the applicationsForRuntime query) and will delegate this to the Runtime Agent. Eventually Compass and Runtime Agent should remove their custom checks and behave in their newly established ways.
2. Runtime Agent periodically fetches all applications from Compass. It should tweak it's reconciliation algorithm a bit and stop comparing application names and rather it should compare by application IDs when trying to determine if an application already exists for example:

   kyma/components/compass-runtime-agent/internal/kyma/util.go

   Line 125 in 6074323

   if runtimeApplication.Name == applicationName {

   • This is needed due to the fact that application names coming from Compass will be unnormalized whereas the Application CR names present in the Kyma runtime will be normalized - a mismatch of names will occur. Also application names from Compass may not be unique, because there might be systems with the same name, but from a different product type. So to solve this we should rely on something more stable - the Application CR currently holds the application ID under .spec.compassMetadata.applicationId.
3. As a result of creation of the Application CR, the Kyma Application Operator (AO) will spawn and create a bunch of other resources based on the Application CR name: HTTPSource CR resource, Application Connectivity Validator, Event Service and Application Gateway. Also from the creation of the HTTPSource CR, the Event Sources Controller will deploy a dedicated HTTP Source Adapter and Knative Channel as well. All these will be prefixed or have the Application CR name somewhere as part of their definitions. Most notably - a Virtual Service with a name matching this pattern <Application CR resource name>-validator will be created as an entry point to the Kyma Runtime mesh for receiving events from actual Applications (the host defined in the Virtual Service is the same host used by Compass when Eventing Configuration is requested by an Application). As of today when an Application paired with Compass requests Eventing Configuration from Compass (in other words "what is the URL I should send business events to") what happens is that Compass manually constructs this URL based on the currently set runtime_eventServiceUrl label for the respective runtime which is configured to receive events from that Application. The runtime_eventServiceUrl label is set by the Runtime Agent and is usually the host specified as the eventing gateway of the runtime. So Compass simply concatenates this like so: <runtime_eventServiceUrl>/appName/v1/events
   • We want to eliminate Compass having to know how to construct this URL - it should be something provided by the actual runtime itself. And in reality Compass will no longer know how to construct this URL as appName will no longer be determinable. appName matches the Application CR name which was used when registering the CR in the Kyma cluster. Compass cannot know (in theory) what normalization has been applied by the Kyma Runtime and (in reality) cannot know what suffix was appended to each Application CR
   • In this sense after all the necessary cluster resources are created and deployed by AO and Event Sources Controller (as a result of the creation of a new Application CR) the Runtime Agent should make sure to call Compass and label the respective runtime with <app_id>_eventURL and the value being the actual absolute /v1/events URL (we could rethink the name of the label, but it should contain the app ID)
   • For backwards compatibility reasons it should be made sure that after introducing this change for all pre-existent Application CRs their respective eventURLs are sent to Compass (maybe the regular update of Application CRs should ensure that)
   • Upon removal of an Application CR from the Kyma runtime the Runtime Agent should ensure that the runtime is unlabeled with the eventURL
   • We will likely have issues with GraphQL which doesn't allow dashes as part of query params etc.. So the dashes in the app ID (which is part of the label key) might have to be underscored and post-processed by Compass later on.

[ghost]

This issue has been automatically marked as stale due to the lack of recent activity. It will soon be closed if no further activity occurs. Thank you for your contributions.

[ghost]

This issue has been automatically marked as stale due to the lack of recent activity. It will soon be closed if no further activity occurs. Thank you for your contributions.

[ghost]

This issue has been automatically marked as stale due to the lack of recent activity. It will soon be closed if no further activity occurs. Thank you for your contributions.

[janmedrek]

Will be handled in: kyma-incubator/reconciler#419