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

gitCredentialsSecret doesn't follow RFC 1123 specification - also not in nightly build #500

Closed
PaulienVa opened this issue Sep 4, 2024 · 6 comments
Labels

Comments

@PaulienVa
Copy link
Contributor

PaulienVa commented Sep 4, 2024

Affected Stackable version

stackable0.0.0-dev

Affected Apache Airflow version

2.9.3

Current and expected behavior

Current behavior:

With the following cluster configuration; the following error will prevent the webserver and scheduler to start up:

create Pod airflow-webserver-default-1 in StatefulSet airflow-webserver-default failed error: 
Pod "airflow-webserver-default-1" is invalid: [spec.containers[2].env[2].valueFrom.secretKeyRef.name: 
Invalid value: "gitCredentialsSecret": a lowercase RFC 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])?)*'), 
spec.containers[2].env[3].valueFrom.secretKeyRef.name: Invalid value: "gitCredentialsSecret": 
a lowercase RFC 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])?)*')]
---
apiVersion: airflow.stackable.tech/v1alpha1
kind: AirflowCluster
metadata:
  name: airflow
spec:
  image:
    productVersion: 2.8.1
  clusterConfig:
    loadExamples: false
    exposeConfig: false
    listenerClass: cluster-internal # https://docs.stackable.tech/home/nightly/concepts/service-exposition
    credentialsSecret: simple-airflow-credentials
    dagsGitSync: 
    - repo: <OUR_PRIVATE_REPO>
      branch: "main" 
      gitFolder: "dags" 
      depth: 10 
      wait: 20 
      credentialsSecret: git-credentials
      gitSyncConf: 
        --rev: HEAD 
        --git-config: http.sslCAInfo:/tmp/ca-cert/ca.crt
    volumeMounts:
    - name: "np-ca"
      mountPath: "/tmp/ca-cert"  
    volumes:
    - name: "np-ca"
      configMap:
        name: "np-ca"
        items: 
        - key: "ca.crt"
          path: "ca.crt"
  webservers:
    roleGroups:
      default:
        replicas: 1
    podOverrides:
      spec:
        containers:
          - name: gitsync-1
            volumeMounts:
            - name: "np-ca"
              mountPath: "/tmp/ca-cert"  
  kubernetesExecutors:
    config:
      resources:
        cpu:
          min: 400m
          max: 800m
        memory:
          limit: 2Gi
    podOverrides:
      spec:
        containers:
          - name: gitsync-1
            volumeMounts:
              - name: "np-ca"
                mountPath: "/tmp/ca-cert"
  schedulers:
    roleGroups:
      default:
        replicas: 1
    podOverrides:
      spec:
        containers:
          - name: gitsync-1
            volumeMounts:
            - name: "np-ca"
              mountPath: "/tmp/ca-cert"

Expected behavior
No error is shown and the webserver and scheduler are starting up.

Fix that is working

---
apiVersion: airflow.stackable.tech/v1alpha1
kind: AirflowCluster
metadata:
  name: airflow
spec:
  image:
    productVersion: 2.9.3
  clusterConfig:
    loadExamples: false
    exposeConfig: false
    listenerClass: external-unstable
    credentialsSecret: simple-airflow-credentials
    dagsGitSync:
      - repo: <OUR_PRIVATE_REPO>
        branch: "main"
        gitFolder: "dags"
        depth: 10
        wait: 20
        credentialsSecret: gitcredentials
        gitSyncConf:
          --rev: HEAD
          --git-config: http.sslCAInfo:/tmp/ca-cert/ca.crt
    volumeMounts:
      - name: "np-ca"
        mountPath: "/tmp/ca-cert"
    volumes:
      - name: "np-ca"
        configMap:
          name: "np-ca"
          items:
            - key: "ca.crt"
              path: "ca.crt"
  webservers:
    roleGroups:
      default:
        replicas: 1
        envOverrides:
          GITSYNC_USERNAME: test
    podOverrides:
      spec:
        containers:
          - name: gitsync-1
            volumeMounts:
              - name: "np-ca"
                mountPath: "/tmp/ca-cert"
            env:
              - name: GITSYNC_PASSWORD
                valueFrom:
                  secretKeyRef:
                    name: gitcredentials
                    key: password
  kubernetesExecutors:
    config:
      resources:
        cpu:
          min: 400m
          max: 800m
        memory:
          limit: 2Gi
      envOverrides:
        GITSYNC_USERNAME: test
      podOverrides:
        spec:
          containers:
          - name: gitsync-1
            volumeMounts:
            - name: "np-ca"
              mountPath: "/tmp/ca-cert"
              env:
              - name: GITSYNC_PASSWORD
                valueFrom:
                  secretKeyRef:
                    name: gitcredentials
                    key: password
  schedulers:
    roleGroups:
      default:
        replicas: 1
        envOverrides:
          GITSYNC_USERNAME: test
    podOverrides:
      spec:
        containers:
          - name: gitsync-1
            volumeMounts:
              - name: "np-ca"
                mountPath: "/tmp/ca-cert"
            env:
              - name: GITSYNC_PASSWORD
                valueFrom:
                  secretKeyRef:
                    name: gitcredentials
                    key: password

Possible solution

It would be nice to to use the podOverrides to be able to start up the airflow cluster

Additional context

Bug already reported: #486

Environment

kubernetes cluster: openshift

Would you like to work on fixing this bug?

None

@adwk67
Copy link
Member

adwk67 commented Sep 4, 2024

Could you attach the Pod description (in yaml) for the airflow scheduler or webserver, showing (at least) the image section and the env section (with values changed where appropriate)? (for the failing case, not the workaround)

@adwk67
Copy link
Member

adwk67 commented Sep 4, 2024

That internal variable gitCredentialsSecret should actually be removed as it is dead code (I should have noticed it when I implemented the fix) and is not called in the nightly version of the operator. Are you sure you are runnning the current nightly version? Can you use pullPolicy: Always just to make sure?
(see https://github.com/search?q=repo%3Astackabletech%2Fairflow-operator+gitCredentialsSecret+&type=code and https://github.com/search?q=repo%3Astackabletech%2Fairflow-operator+%22GIT_CREDENTIALS_SECRET_PROPERTY%22&type=code).

@PaulienVa
Copy link
Contributor Author

I asked our ops engineers a few times, but will do it again, to make sure we have the correct nightly, coming back to you about it

@PaulienVa
Copy link
Contributor Author

@adwk67 somehow it is working now - something went wrong on our side. Sorry for the discussion. Closing this ticket.

@chdudek
Copy link

chdudek commented Oct 7, 2024

Just asking for clarification:

This used to work in 23.4 and is not working in 23.7 anymore and will be fixed in the next release?

If so, shouldn't there be an information on the 23.7 release notes that there is a known breaking issue for Airflow?

@adwk67
Copy link
Member

adwk67 commented Oct 7, 2024

This was working in 24.3, was broken in 24.7, has since been fixed and is available in the nightly builds. But, yes, you're right: it's a bit confusing so I'll add something to the 24.7 release notes as per your suggestion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants