Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Zarf (helm) ignores strict schema decoding on deploy #2548

Open
mjnagel opened this issue May 24, 2024 · 2 comments
Open

Zarf (helm) ignores strict schema decoding on deploy #2548

mjnagel opened this issue May 24, 2024 · 2 comments

Comments

@mjnagel
Copy link
Contributor

mjnagel commented May 24, 2024

Environment

Device and OS: macOS
App version: v0.34.0
Kubernetes distro being used: k3d

Steps to reproduce

Reproduction is using a k3s custom resource, so this should be reproducable on a k3d cluster easily.

  1. Make a manifest for a k3s addon. Do not follow the schema. Example:
    apiVersion: k3s.cattle.io/v1
    kind: Addon
    metadata:
      name: test
      namespace: default
    spec:
      noSchema: here
  2. Attempt to kubectl apply the manifest. You will see an error like Error from server (BadRequest): error when creating "addon.yaml": Addon in version "v1" cannot be handled as a Addon: strict decoding error: unknown field "spec.noSchema".
  3. Make a zarf package with this manifest, example:
    kind: ZarfPackageConfig
    metadata:
      name: test
    components:
      - name: test
        required: true
        manifests:
          - name: test
            files:
              - "addon.yaml"
  4. Create and deploy this zarf package.

Expected result

Zarf package would fail to deploy with the same schema error.

Actual Result

Zarf package successfully deploys, resulting in a bad CR in cluster.

Visual Proof (screenshots, videos, text, etc)

Logs
k apply -f addon.yaml
Error from server (BadRequest): error when creating "addon.yaml": Addon in version "v1" cannot be handled as a Addon: strict decoding error: unknown field "spec.noSchema"zarf p deploy build/zarf-package-test-amd64.tar.zst --confirm

 NOTE  Using config file /Users/mjnagel/defenseunicorns/uds-core/zarf-config.yaml

 NOTE  Saving log file to
       /var/folders/56/3lg0nwq57ld8_xv7cjm9zjgh0000gn/T/zarf-2024-05-24-13-58-03-1643861909.log
  •  Loading package from "build/zarf-package-test-amd64.tar.zst"
  •  Loading package from "build/zarf-package-test-amd64.tar.zst"
  •  Loading package from "build/zarf-package-test-amd64.tar.zst"


                                                                                                      
  📦 PACKAGE DEFINITION                                                                               
                                                                                                      


kind: ZarfPackageConfig

━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
metadata:  information about this package

  name: test
  architecture: amd64
  aggregateChecksum: 430e9fc32d337d679893f96627f20fe7109f448ad9fa4e24671486270b7c0d9f

━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
build:  info about the machine, zarf version, and user that created this package

  terminal: hargrave.local
  user: mjnagel
  architecture: amd64
  timestamp: Fri, 24 May 2024 13:55:00 -0600
  version: v0.34.0
  migrations:
  - scripts-to-actions
  - pluralize-set-variable
  lastNonBreakingVersion: v0.27.0

━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
components:  components selected for this operation

- name: test
  required: true
  manifests:
  - name: test
    files:
    - addon.yaml

━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

  ✔  Deploy Zarf package confirmed
  •  Waiting for cluster connection
  •  Waiting for cluster connection
  •  Gathering additional cluster information (if available)
  •  Gathering additional cluster information (if available)

                                                                                                      
  📦 TEST COMPONENT                                                                                   
                                                                                                      

  •  Loading the Zarf State from the Kubernetes cluster
  •  Loading the Zarf State from the Kubernetes cluster
  •  Starting helm chart generation test
  •  Starting helm chart generation test
  •  Processing helm chart raw-test-test-test:0.1.1716580683 from Zarf-generated helm chart
  •  Processing helm chart raw-test-test-test:0.1.1716580683 from Zarf-generated helm chart
  ✔  Zarf deployment completek get addon test -o yaml
apiVersion: k3s.cattle.io/v1
kind: Addon
metadata:
  annotations:
    meta.helm.sh/release-name: zarf-5dd7fba6079f881e678be27187fc0f76aec9d1f8
    meta.helm.sh/release-namespace: default
  creationTimestamp: "2024-05-24T19:58:04Z"
  generation: 1
  labels:
    app.kubernetes.io/managed-by: Helm
  name: test
  namespace: default
  resourceVersion: "9237"
  uid: 2f5ac0c1-562b-4af6-bdc1-a19abae204f5
spec: {}

Severity/Priority

This feels relatively important since it could lean to unknown states if operators are not built to handle non-schema adhering custom resources.

Additional Context

It appears that this may be an upstream helm issue, I was able to reproduce the same behavior just with a basic helm chart. See helm/helm#12470 which seems to be tracking this problem. Unsure that it would make sense to do anything on the zarf side to handle this, but wanted to open the issue for tracking/discussion. Unsure if other projects like flux have encountered this issue and handled it or if everything is waiting on helm upstream.

@mjnagel mjnagel changed the title Zarf ingores openapi schema Zarf (helm) ignores openapi schema May 24, 2024
@mjnagel mjnagel changed the title Zarf (helm) ignores openapi schema Zarf (helm) ignores openapi schema validation on deploy May 24, 2024
@mjnagel
Copy link
Contributor Author

mjnagel commented May 24, 2024

After further reviewing the helm behavior this appears to be slightly less bad than originally expected - helm does enforce the schema for required fields/types but does not prohibit/restrict unknown fields from being added. This is different from default kubectl apply behavior which enforces strict decoding.

mjnagel added a commit to defenseunicorns/uds-core that referenced this issue May 24, 2024
## Description

Fixes the `Package` CRs to have the correct name in the spec.
## Related Issue

Note that this was not caught earlier due to
zarf-dev/zarf#2548

## Type of change

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Other (security config, docs update, etc)

## Checklist before merging

- [x] Test, docs, adr added or updated as needed
- [x] [Contributor Guide
Steps](https://github.com/defenseunicorns/uds-template-capability/blob/main/CONTRIBUTING.md)(https://github.com/defenseunicorns/uds-template-capability/blob/main/CONTRIBUTING.md#submitting-a-pull-request)
followed
@mjnagel mjnagel changed the title Zarf (helm) ignores openapi schema validation on deploy Zarf (helm) ignores strict schema decoding on deploy May 24, 2024
@mjnagel
Copy link
Contributor Author

mjnagel commented Jun 17, 2024

Updating to link a different upstream helm issue helm/helm#13053

rjferguson21 pushed a commit to defenseunicorns/uds-core that referenced this issue Jul 11, 2024
## Description

Fixes the `Package` CRs to have the correct name in the spec.
## Related Issue

Note that this was not caught earlier due to
zarf-dev/zarf#2548

## Type of change

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Other (security config, docs update, etc)

## Checklist before merging

- [x] Test, docs, adr added or updated as needed
- [x] [Contributor Guide
Steps](https://github.com/defenseunicorns/uds-template-capability/blob/main/CONTRIBUTING.md)(https://github.com/defenseunicorns/uds-template-capability/blob/main/CONTRIBUTING.md#submitting-a-pull-request)
followed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

No branches or pull requests

1 participant