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

Failure to execute helm template when using spec.source.helm.valuesObject #14334

Closed
asaf400 opened this issue Jul 4, 2023 · 11 comments
Closed
Labels
bug Something isn't working

Comments

@asaf400
Copy link

asaf400 commented Jul 4, 2023

I have attempted to use the newly merged feature #11538,

I have an application that has used spec.source.helm.values: | as a string,
Upon redeploying argocd using the latest rc image which includes the merged PR v2.8.0-rc1 (quay.io/argoproj/argocd:v2.8.0-rc1)
I have attempted to modify the application in-place using argocd ui, but changing values: | to valuesObject I have encountered an unexpected error:

Unable to save changes: application spec for openshift-console is invalid: InvalidSpecError: Unable to generate manifests in : rpc error: code = Unknown desc = Manifest generation error (cached): 
`helm template . --name-template openshift-console --namespace openshift-console --kube-version 1.23 --api-versions all.of.k8s/v0/apis.here --include-crds` failed exit status 1: 
Error: execution error at (openshift-console/templates/deployment.yaml:1:70): .Values.bridgeEnvironmentVariables.BRIDGE_BASE_ADDRESS is required. Use --debug flag to render out invalid YAML

The chart I've used is my own helm for okd|openshift console deployment
Within the chart's app deployment template I use this templating rule to enforce a required value:

{{- $name := .Values.bridgeEnvironmentVariables.BRIDGE_BASE_ADDRESS | required ".Values.bridgeEnvironmentVariables.BRIDGE_BASE_ADDRESS is required." -}}

https://github.com/asaf400/openshift-console-chart/blob/main/charts/console/templates/deployment.yaml#L1

It works fine with values as a a string, but failed on that line when using valuesObject
I tried digging into this PR to see what would cause this, but I can't see where potentially the issue would be..

For completeness, this is a sample of the values (already into the .values key):

        bridgeEnvironmentVariables:
          BRIDGE_BASE_ADDRESS: https://console.somedomain.here.dom
          BRIDGE_USER_AUTH_OIDC_CLIENT_ID: deface33-defa-cede-face-defacedeface
          BRIDGE_USER_AUTH_OIDC_ISSUER_URL: https://oauth.id.authproviderdomain.com/
        extraEnv:
          - name: BRIDGE_INACTIVITY_TIMEOUT
            value: "3600"
          - name: BRIDGE_CUSTOM_PRODUCT_NAME
            value: "Tab Title here"
        ingress:
          hosts:
            - host: console.somedomain.here.dom # value will be replaced by a patch
              paths:
                - path: /*
                  pathType: ImplementationSpecific
          sslRedirect: true
@asaf400 asaf400 added the bug Something isn't working label Jul 4, 2023
@blakepettersson
Copy link
Member

I cannot repro this. When I apply the manifest below

apiVersion: argoproj.io/v1alpha1
kind: Application
metadata:
  name: test-asaf
  namespace: argocd
spec:
  syncPolicy:
    automated:
      prune: true
    syncOptions:
      - CreateNamespace=true
  destination:
    server: https://kubernetes.default.svc
    namespace: guestbook
  project: default
  sources:
  - repoURL: https://github.com/asaf400/openshift-console-chart
    targetRevision: main
    path: charts/console
    helm:
      valuesObject:
        bridgeEnvironmentVariables:
          BRIDGE_BASE_ADDRESS: https://console.somedomain.here.dom
          BRIDGE_USER_AUTH_OIDC_CLIENT_ID: deface33-defa-cede-face-defacedeface
          BRIDGE_USER_AUTH_OIDC_ISSUER_URL: https://oauth.id.authproviderdomain.com/
        extraEnv:
          - name: BRIDGE_INACTIVITY_TIMEOUT
            value: "3600"
          - name: BRIDGE_CUSTOM_PRODUCT_NAME
            value: "Tab Title here"
        ingress:
          hosts:
            - host: console.somedomain.here.dom # value will be replaced by a patch
              paths:
                - path: /*
                  pathType: ImplementationSpecific
          sslRedirect: true

I get this

Screenshot 2023-07-18 at 15 31 44

I'll give it a shot when converting from values -> valuesObject to see if I get any different results.

@blakepettersson
Copy link
Member

Nope, same result there.

@asaf400
Copy link
Author

asaf400 commented Jul 18, 2023

I actually revisited this Yesterday, I ran locally using updated master branch,
And I also couldn't reproduce it locally, but I copied the 100% same manifest from local, to running Argo CD instance on EKS cluster, and it happened again - the error, using 2.8.0-rc3*, and Today, I'm going to retry in EKS using current master branch docker image.

Something very odd is going on, since I tried it multiple different ways, copying the entire app k8s object,
copying just the values, in any test cast, the deployed version (2.8.0-rc3*) is production an UNMARSH JSON error

I am suspecting something is going on with the annotations within the values, because that is a string value containing a JSON

@asaf400
Copy link
Author

asaf400 commented Jul 18, 2023

in my original message, I missed the ingress resource key:

      valuesObject:
        ingress:
          annotations:
            alb.ingress.kubernetes.io/actions.ssl-redirect: '{"Type": "redirect",
              "RedirectConfig": { "Protocol": "HTTPS", "Port": "443", "StatusCode":
              "HTTP_301"}}'
            alb.ingress.kubernetes.io/group.name: internal
            alb.ingress.kubernetes.io/healthcheck-path: /api/health
            alb.ingress.kubernetes.io/listen-ports: '[{"HTTP": 80}, {"HTTPS": 443}]'
            alb.ingress.kubernetes.io/load-balancer-attributes: idle_timeout.timeout_seconds=600
            alb.ingress.kubernetes.io/scheme: internet-facing
            alb.ingress.kubernetes.io/target-type: ip
          className: alb
          enabled: true
          hosts:
          - host: hostname.here
            paths:
            - path: /*
              pathType: ImplementationSpecific
          sslRedirect: true
        oidcIdentityProvider: jumpcloud
        secret:
          create: false
        verbosity: 10

note the alb.ingress.kubernetes.io/actions.ssl-redirect
and alb.ingress.kubernetes.io/listen-ports

@asaf400
Copy link
Author

asaf400 commented Jul 18, 2023

I'll try to run thought both troubleshooting \ debugging methods and see if I can spot anything:
https://argo-cd.readthedocs.io/en/stable/developer-guide/debugging-remote-environment/

So far, I've been using this:
https://argo-cd.readthedocs.io/en/stable/developer-guide/running-locally/#run-argo-cd-outside-of-kubernetes

And couldn't reproduce it..

@asaf400
Copy link
Author

asaf400 commented Jul 18, 2023

I was homing in on these lines:

if err := json.Unmarshal(data, &v); err != nil {

by following the tests, even tough the test flow, doesn't invoke helm template

assert.Equal(t, testCase.expectValue, source.ValuesString())

@asaf400
Copy link
Author

asaf400 commented Jul 19, 2023

Found the issue,
I've been mixing versions, new image, and running old chart..

Because we really needed the feature, I jumped the gun, and therefore, I've been Biting the bullet :/
I'll describe this in more details tomorrow.. (and close the issue)

@blakepettersson
Copy link
Member

@asaf400 did you still want to add details to this ticket and/or should this be closed?

@asaf400
Copy link
Author

asaf400 commented Jul 24, 2023

Because I was still using the released chart version,
which is still v2.7.8, And not v2.8.0 Which is the version target of the valuesObject feature, and the CRDs are part of the chart,
I was running on 'stable' released CRDs.

Now at the starting point of trying the valuesObject feature, I did update the CRDs ot the RC version,
but at somepoint I had to modify the Argo CD configmap and so I updated my helm release, which in-turn re-synced the CRDs to the stable version, which I did not realize at the time..

And since I only updated the deployed argo cd image itself to the RC, I ran 'incompatible' versions,
Similar to how you'd run an app against a non-migrated \ misaligned DB Schema.

Using Telepresence, with adjustments,
I was able to debug the repo-server locally and expose with breakpoints the real error,
which wasn't raised to the user..
I did not save the message, but it was basically the Application object given to repo-server was already missing the valuesObject field...

.Values.bridgeEnvironmentVariables.BRIDGE_BASE_ADDRESS is required is sympthom of Argo CD running the 2.8.0 rc and the k8s api being able to accept the Application with valuesObject which the CRD doesn't specify that field,
then Argo or k8s just nullify that field, and so the chart is deployed basically without any values which prompts that error message from within the template..

This proves that Argo CD currently as-it-stands doesn't verify the CRDs spec \ versions,
and when implementing the valuesObject feature, I would recomment changing the api version of argoproj.io/v1alpha1 for kind: Application to something like argoproj.io/v1alpha2 maybe..
That way Argo 2.8 can strictly understand which CRD version is which application and the features of the CRD..

@crenshaw-dev
Copy link
Collaborator

@asaf400 yep, this is a downside of how we've chosen to version our CRDs (i.e. to not version them). Our policy is to add fields only, supporting old fields in a backwards-compatible way. But we do expect that users upgrade the CRDs in-place over the existing version.

It's possible that we'll take more advantage of API versioning in the future. But for now, there are no plans to change.

@asaf400 asaf400 closed this as completed Jul 24, 2023
@asaf400
Copy link
Author

asaf400 commented Jul 24, 2023

Thank you @blakepettersson for taking the time and trying the chart on your setup, and confirm it works..
And thanks @crenshaw-dev for the reply.

🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Backlog
Development

No branches or pull requests

3 participants