Skip to content

capi: allow configuring Ansible Galaxy installs - #2081

Open
MaxRink wants to merge 7 commits into
kubernetes-sigs:mainfrom
MaxRink:tcaas/ansible-galaxy-install-options
Open

capi: allow configuring Ansible Galaxy installs#2081
MaxRink wants to merge 7 commits into
kubernetes-sigs:mainfrom
MaxRink:tcaas/ansible-galaxy-install-options

Conversation

@MaxRink

@MaxRink MaxRink commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

What this PR does / why we need it

make deps currently installs required Ansible collections directly from the default Ansible Galaxy configuration. That makes it hard to use image-builder in environments that require a Galaxy mirror, private Automation Hub, pre-warmed collection cache, custom timeout, or offline collection installation.

This adds a small shared helper for CAPI dependency scripts so collection installs can be configured through environment variables while keeping the default behavior unchanged.

Supported variables:

  • ANSIBLE_GALAXY_SERVER
  • ANSIBLE_GALAXY_TOKEN
  • ANSIBLE_GALAXY_IGNORE_CERTS=true
  • ANSIBLE_GALAXY_TIMEOUT
  • ANSIBLE_GALAXY_COLLECTIONS_PATH
  • ANSIBLE_GALAXY_NO_CACHE=true
  • ANSIBLE_GALAXY_OFFLINE=true

The PR intentionally keeps this generic. It does not add any project-specific mirror URLs, credentials, or cache layout assumptions.

Which issue(s) this PR fixes

None

Testing

  • bash -n images/capi/hack/utils.sh images/capi/hack/ensure-ansible.sh images/capi/hack/ensure-s3.sh
  • git diff --check
  • shellcheck -x -P images/capi images/capi/hack/utils.sh images/capi/hack/ensure-ansible.sh images/capi/hack/ensure-s3.sh
  • /bin/bash 3.2 stubbed ansible-galaxy regression check with no ANSIBLE_GALAXY_* variables set
  • /bin/bash 3.2 stubbed ansible-galaxy argument check covering server, token, ignore-certs, timeout, collections-path, no-cache, and offline options

@kubernetes-prow

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign jsturtevant for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubernetes-prow kubernetes-prow Bot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 26, 2026
@drew-viles

Copy link
Copy Markdown
Contributor

/retest

@MaxRink
MaxRink force-pushed the tcaas/ansible-galaxy-install-options branch 3 times, most recently from e6d7a63 to 1014184 Compare July 1, 2026 17:58
Comment thread images/capi/hack/utils.sh Outdated
@MaxRink
MaxRink force-pushed the tcaas/ansible-galaxy-install-options branch 2 times, most recently from a8c671f to 4a6c90f Compare July 5, 2026 12:46

@drew-viles drew-viles left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems logical to me!

/ok-to-test

@kubernetes-prow kubernetes-prow Bot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Jul 6, 2026
@MaxRink
MaxRink force-pushed the tcaas/ansible-galaxy-install-options branch 3 times, most recently from 5050799 to 5994aa5 Compare July 7, 2026 23:01

@drew-viles drew-viles left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor notes actually.

Comment thread images/capi/hack/utils.sh Outdated
Comment thread images/capi/hack/utils.sh
@MaxRink
MaxRink force-pushed the tcaas/ansible-galaxy-install-options branch from 5994aa5 to 0f672d1 Compare July 17, 2026 16:15
@drew-viles

Copy link
Copy Markdown
Contributor

/retest

@mboersma

Copy link
Copy Markdown
Contributor

/label tide/merge-method-squash

@kubernetes-prow kubernetes-prow Bot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Jul 19, 2026
@MaxRink
MaxRink force-pushed the tcaas/ansible-galaxy-install-options branch from 0f672d1 to c1abd49 Compare August 9, 2026 14:45

@mboersma mboersma left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we address a few propagation and security issues before merging?

Galaxy credentials may leak through tracing/process arguments, custom install paths aren’t always added to Ansible’s runtime search path, make can evaluate $() in environment-provided paths, and the GCE su - packer flow drops the new Galaxy variables.

@linux-foundation-easycla

linux-foundation-easycla Bot commented Aug 11, 2026

Copy link
Copy Markdown

CLA Signed
The committers listed above are authorized under a signed CLA.

@kubernetes-prow kubernetes-prow Bot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 11, 2026
@MaxRink
MaxRink force-pushed the tcaas/ansible-galaxy-install-options branch from 4439327 to eb020b7 Compare August 12, 2026 00:59
@kubernetes-prow kubernetes-prow Bot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Aug 12, 2026
@kubernetes-prow kubernetes-prow Bot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. and removed cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Aug 22, 2026
@MaxRink

MaxRink commented Aug 22, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the detailed feedback. I addressed all four propagation/security points in commit bba786d7912d1d46c91bd80d7dea9d6ba59071bc:

  1. Galaxy credential handlingimages/capi/hack/utils.sh now keeps xtrace disabled for the complete credential lifecycle, writes ANSIBLE_GALAXY_TOKEN through stdin to a 0600 temporary token file using a race-safe mktemp path, removes the token from child environments, never passes it as --token, and cleans up the file.
  2. Runtime collection discoveryimages/capi/hack/utils.sh exports ANSIBLE_COLLECTIONS_PATH with the custom Galaxy path, and images/capi/Makefile reliably prepends it to any existing runtime path.
  3. Literal Make paths — the Make assignments now use raw $(value ...) values with a separate override/export, so environment-provided $() sequences remain literal and cannot be evaluated.
  4. GCE su - packer propagation — both existing GCE scripts retain the complete Galaxy variable whitelist, including ANSIBLE_COLLECTIONS_PATH and ANSIBLE_GALAXY_TOKEN_PATH; focused tests now assert this for both flows.

Changed files:

  • images/capi/hack/utils.sh
  • images/capi/Makefile
  • images/capi/hack/utils_test.py
  • docs/book/src/capi/capi.md

Validation:

  • make --no-print-directory -C images/capi test-galaxy
  • python3 -m unittest discover -s images/capi/packer/qemu/scripts -p '*_test.py'
  • shellcheck images/capi/hack/utils.sh
  • ruff check --ignore EXE001 images/capi/hack/utils_test.py
  • ruff format --check images/capi/hack/utils_test.py
  • Bash syntax checks and git diff --check

The broader shellcheck invocation still reports pre-existing warnings in unchanged CI/helper code (SC1091, SC2155, SC2086). Please re-review when convenient.

@MaxRink
MaxRink force-pushed the tcaas/ansible-galaxy-install-options branch from bba786d to e30d0cd Compare August 22, 2026 16:37
@kubernetes-prow kubernetes-prow Bot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Aug 22, 2026
@MaxRink

MaxRink commented Aug 22, 2026

Copy link
Copy Markdown
Contributor Author

Follow-up on EasyCLA: the failure was caused by the Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> trailer in the just-added commit. EasyCLA explicitly reported @Copilot as missing CLA authorization; PR #2100's passing commits contain only the authorized MaxRink author/committer metadata.

I amended only that commit message (no code/tree changes), preserving Maximilian Rink as author and committer. The final commit is e30d0cdf3ffcbd4c7b514c156c33259f59a7dc55, and EasyCLA now reports pass. The prior implementation and review reply remain unchanged.

@MaxRink

MaxRink commented Aug 22, 2026

Copy link
Copy Markdown
Contributor Author

/test pull-ova-all

Signed-off-by: Maximilian Rink <maximilian.rink@telekom.de>
Address review feedback on the Ansible Galaxy install helper:

- Suspend xtrace around both the ANSIBLE_GALAXY_TOKEN condition check
  and the final ansible-galaxy invocation in
  ansible_galaxy_collection_install, restoring the previous xtrace
  state afterward. Without this, DEBUG=1 ANSIBLE_GALAXY_TOKEN=...
  make deps wrote the token to the build log.
- When ANSIBLE_GALAXY_COLLECTIONS_PATH is set, also export the
  standard ANSIBLE_COLLECTIONS_PATH so Ansible can discover the
  installed collections at provisioning time, not just during the
  ansible-galaxy install step.

Signed-off-by: Maximilian Rink <maximilian.rink@telekom.de>
Prevent token exposure via process args, make custom collection paths available at runtime without evaluating env-provided \ in make, and preserve Galaxy-related env vars across GCE su - packer runs.
@MaxRink
MaxRink force-pushed the tcaas/ansible-galaxy-install-options branch from e30d0cd to 1932d48 Compare August 28, 2026 00:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants