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

Fix wrong values set for JOB_KUBE_MAIN_CONTAINER_IMAGE_PULL_SECRET ConfigMap key #383

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

jamezrin
Copy link

@jamezrin jamezrin commented Jan 22, 2025

What

Fixes a regression caused by 201b8e8 (causes issue: airbytehq/airbyte#52034)

When specifying common values in global.imagePullSecrets and global.jobs.kube.main_container_image_pull_secret, the duplicates would cause the application to fail at runtime.

Also, when not specifying any pull secrets (either via global.imagePullSecrets or global.jobs.kube.main_container_image_pull_secret), an empty string would be set for the value of the JOB_KUBE_MAIN_CONTAINER_IMAGE_PULL_SECRET configmap key, causing to end up as null in the configmap.

How

  • Dedupes the names of the pull secrets before joining them, using the uniq function
  • Quotes the values, using the quote function. If the result of the concatenation of the list resolves to an empty string, this way the entry will still end up in the configmap. Otherwise, it will end up being interpreted as null and will actually not be present in the configmap.

Can this PR be safely reverted and rolled back?

  • YES 💚
  • NO ❌

…CONTAINER_IMAGE_PULL_SECRET

Fixes a regression caused by #52034

When specifying the common values in `global.imagePullSecrets` and
`global.jobs.kube.main_container_image_pull_secret`, the duplicate would
cause the application to fail at runtime.

Also, when not specifying any pull secrets, an empty string would be set
for the value of the `JOB_KUBE_MAIN_CONTAINER_IMAGE_PULL_SECRET` configmap
key, causing to end up as `null` in the configmap.
@CLAassistant
Copy link

CLAassistant commented Jan 22, 2025

CLA assistant check
All committers have signed the CLA.

@marcosmarxm
Copy link
Member

Thanks @jamezrin the team is working to get a fix for #52034 asap.

@abuchanan-airbyte
Copy link
Contributor

/create-oss-pr

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

Successfully merging this pull request may close these issues.

5 participants