apps-cli-plugin icon indicating copy to clipboard operation
apps-cli-plugin copied to clipboard

Can set booleans (`--live-update` and/or `--debug`) to a non-boolean value via workload.yaml

Open heyjcollins opened this issue 2 years ago • 3 comments

Please fill out the issue checklist below and provide ALL the requested information.

  • [X] I reviewed open and closed Github issues that may be related to my problem.
  • [X] I am reporting a bug that others will be able to reproduce.

Describe the bug

There are currently two boolean flags that can be set via command flags and/or workload.yaml:

  • --live-update
  • --debug The apps plugin validates the values passed via command flags today and will throw an error if a non-boolean value is provided (e.g. --debug foo will fail validation).

However, it's possible to set spec.params.debug or spec.params.live-update to a value of foo and submit the workload.yaml without error.

Expected behavior

apps plugin should throw an error if/when workload.yaml includes non-boolean values for boolean workload spec properties.

Steps to Reproduce

  1. create a workload using the following workload.yaml
    ---
    apiVersion: carto.run/v1alpha1
    kind: Workload
    metadata:
      labels:
        app.kubernetes.io/part-of: pc
        apps.tanzu.vmware.com/workload-type: web
      name: pc
      namespace: default
    spec:
      params:
      - name: live-update
        value: foo
      - name: debug
        value: bar
      source:
        git:
          ref:
            tag: tap-1.1
          url: https://github.com/sample-accelerators/spring-petclinic
    

Version (Apps plugin version, Version of K8s running on cluster)

Put the output of the following command

tanzu version && tanzu apps version version: v0.11.2 buildDate: 2022-03-17 sha: 9f16f375 v0.5.1

Environment where the bug was observed (cloud, OS, etc)

Additional context & Relevant Debug Output (Logs, etc)

cc @shashwathi (Hey Shash - I ended up logging this bug since I didn't see it this morning).

heyjcollins avatar May 02 '22 19:05 heyjcollins

Had some additional offline discussions with @scothis regarding this issue. We're using the params as a vehicle to enable/disable debug/live-update. It would be better to namespace these params if we're going to apply validation as described here. Namespaced params examples:

  • debug --> apps.tanzu.vmware.com/debug
  • live-update --> apps.tanzu.vmware.com/live-update
  • annotations --> apps.tanzu.vmware.com/annotations

Any changes to the params must be coordinated with corresponding changes to Cartographer supply chain(s). We should hold off on addressing this issue until/unless we've done that.

heyjcollins avatar May 02 '22 23:05 heyjcollins

label blocked applied because we need to namespace the labels before we can reasonably apply this custom boolean validation.

heyjcollins avatar May 06 '22 22:05 heyjcollins

Not migrated: it's potentially blocked on changes to carto v1 (if we move to using tanzu namespaced versions of the live-update and debug params, eg. live-update --> apps.tanzu.vmware.com/live-update ). Although this will add some polish and prevent user error, the value in relation to overall priorities on the plate is questionable.

heyjcollins avatar Dec 07 '23 19:12 heyjcollins