Workload allowed to be created with invalid name

[odinnordico]

Bug description:

Cartographer allow to create workloads with names that does not complain with all possible components in the supply chain, being able to create a workload with name with dot(.) in it, but failing when try to create a knative service using it as CNR.

Steps to reproduce:

Having a TAP 1.0.0 cluster configured with full profile and using tanzu apps plugin to create resources in the supply chain:

1. Create a yaml file (tanzu-java-web-app-2.6.yaml) with name containing dots (.)

apiVersion: carto.run/v1alpha1
kind: Workload
metadata:
  name: tanzu-java-web-app-2.6
  labels:
    app.kubernetes.io/part-of: tanzu-java-web-app
    apps.tanzu.vmware.com/workload-type:  web
spec:
  source:
    git:
      url: https://github.com/odinnordico/tanzu-java-web-app
      ref:
        branch: spring-boot-2.6

2. Create the workload without setting the name in the cli: tanzu apps workload create -y --tail --wait --tail-timestamp -f tanzu-java-web-app-2.6.yaml
3. Wait for some minutes while the workload is processed, then inspect the corresponding app created: kubectl get apps.kappctrl.k14s.io

NAME                                   DESCRIPTION                                                                                                             SINCE-DEPLOY   AGE
tanzu-java-web-app-2.6   Reconcile failed: Deploying: Error (see .status.usefulErrorMessage for details)   3s                       
  25m

4. Check the Useful Error Message: kubectl describe apps.kappctrl.k14s.io tanzu-java-web-app-2.6

Useful Error Message:  kapp: Error: Applying create service/tanzu-java-web-app-2.6 (serving.knative.dev/v1) namespace: default:
  Creating resource service/tanzu-java-web-app-2.6 (serving.knative.dev/v1) namespace: default: admission webhook "validation.webhook.serving.knative.dev" denied the request: validation failed: not a DNS 1035 label:
  - a DNS-1035 label must consist of lower case alphanumeric characters or '-'
  - start with an alphabetic character
  - and end with an alphanumeric character (e.g. 'my-name',  or 'abc-123', regex used for validation is '[a-z]([-a-z0-9]*[a-z0-9])?')

5. List the available workloads: tanzu apps workload list

NAME                                  APP                              READY   AGE
tanzu-java-web-app-2.6   tanzu-java-web-app   Ready   26m

Expected behavior:

Cartographer should not allow to create resource with names which are not aligned with itself, this creation intent must be rejected with an useful error

Actual behavior:

Resource with not aligned name is created but fails in following supply chain steps

Logs, k8s object dumps:

Versions:

• Infrastructure: GKE
• cartographer version: 0.1.0
• k8s version: 1.21
• cnrs version: 1.1.0
• knative version 1.0.1

Deployment info:

Check Steps

Additional context:

[github-actions[bot]]

Hello @odinnordico! Welcome to the Cartographer community and thank you for opening your first issue, we appreciate your contributions

[tbr11]

Considering two options:

• Become more strict in accepting the name (alphanumeric and dashes?)
• Throw a warning in status

[cirocosta]

Hey @odinnordico , thanks for reporting!

I was trying to gather some more context here, and end up looking at what knative is making use of for that check - https://github.com/knative/serving/blob/d5d489c0babd5c9ae9a37d707c06df92d70711f5/vendor/knative.dev/pkg/apis/metadata_validation.go#L37-L47 seems quite relevant.

Given that the name of the Workload/Deliverable/etc is made part of the children labelset

https://github.com/vmware-tanzu/cartographer/blob/ba558d67e10073a926205e0d0f86f201cf34a981/pkg/realizer/workload/component.go#L87-L94

it might be worth at least performing such validation in the name to prevent further problems w/ setting them in labelsets (although +1 on having a bigger discussion on to what degree we should be strict about it).

It's tricky that template authors are able to make use of the name wherever they want (through #@ workload.metadata.name or $(workload.metadata.name)$, but I imagine that with a smaller, more restrictive naming scheme, it might reduce the possible amount of mistakes one can make)

---

see https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#syntax-and-character-set

Valid label value:

- must be 63 characters or less (can be empty),
- unless empty, must begin and end with an alphanumeric character ([a-z0-9A-Z]),
- could contain dashes (-), underscores (_), dots (.), and alphanumerics between.

[emmjohnson]

PR sent out months ago. Was held for possible breaking changes. Need to review at next pre/IPM.

[karayim]

Block on upgrade documentation to describe potential breaking change.

[emmjohnson]

We should be following - https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#dns-label-names

[squeedee]

Acceptance Criteria:

[ ] Check behviour is only on 'Edit' for workload/template [ ] Document in OSS docs for template/workload [ ] Check code for pre/post upgrade awareness [ ] Document what happens on upgrade, and how to deal with it if you upgrade without patching first (include error that occurs when trying to edit)