Skip to content

Commit 340dcd1

Browse files
fix(ci): address review feedback for PR #1884
- Use least-privilege secrets in ci.yaml: enumerate specific secrets for e2e-tests instead of secrets:inherit; remove secrets:inherit from release job (only uses automatic GITHUB_TOKEN) - Add permissions (contents:read, id-token:write) to forward-compatibility run-e2e-tests job - Enumerate secrets in forward-compatibility.yaml for consistency - Fix container-toolkit GitHub repo name (NVIDIA/nvidia-container-toolkit) - Fix mig-manager GitHub repo name (NVIDIA/mig-parted, not k8s-mig-manager) - Replace yq-based YAML merge with Helm native multi-file values (-f file1 -f file2) since remote test machines lack yq - Pass OPERATOR_OPTIONS in values-file helm install path to avoid silently dropping test-case-specific flags Co-authored-by: Rajath Agasthya <rajathagasthya@users.noreply.github.com> Signed-off-by: Carlos Eduardo Arango Gutierrez <eduardoa@nvidia.com>
1 parent d4e21ac commit 340dcd1

4 files changed

Lines changed: 27 additions & 19 deletions

File tree

.github/scripts/get-latest-images.sh

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,15 @@ fi
3232
case "${COMPONENT}" in
3333
toolkit)
3434
IMAGE_REPO="ghcr.io/nvidia/container-toolkit"
35-
GITHUB_REPO="NVIDIA/container-toolkit"
35+
GITHUB_REPO="NVIDIA/nvidia-container-toolkit"
3636
;;
3737
device-plugin)
3838
IMAGE_REPO="ghcr.io/nvidia/k8s-device-plugin"
3939
GITHUB_REPO="NVIDIA/k8s-device-plugin"
4040
;;
4141
mig-manager)
4242
IMAGE_REPO="ghcr.io/nvidia/k8s-mig-manager"
43-
GITHUB_REPO="NVIDIA/k8s-mig-manager"
43+
GITHUB_REPO="NVIDIA/mig-parted"
4444
;;
4545
*)
4646
echo "Error: Unknown component '${COMPONENT}'" >&2

.github/workflows/ci.yaml

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,10 @@ jobs:
8080
with:
8181
operator_image: ${{ needs.variables.outputs.operator_image }}
8282
operator_version: ${{ needs.variables.outputs.operator_version }}
83-
secrets: inherit
83+
secrets:
84+
AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }}
85+
AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }}
86+
AWS_SSH_KEY: ${{ secrets.AWS_SSH_KEY }}
8487

8588
release:
8689
if: github.event_name == 'push' && github.ref == 'refs/heads/main'
@@ -92,4 +95,3 @@ jobs:
9295
with:
9396
commit_short_sha: ${{ needs.variables.outputs.commit_short_sha }}
9497
operator_image_base: ${{ needs.variables.outputs.operator_image_base }}
95-
secrets: inherit

.github/workflows/forward-compatibility.yaml

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,12 +69,20 @@ jobs:
6969

7070
run-e2e-tests:
7171
needs: [fetch-latest-images]
72+
permissions:
73+
contents: read
74+
id-token: write
7275
uses: ./.github/workflows/e2e-tests.yaml
7376
with:
7477
operator_image: ghcr.io/nvidia/gpu-operator
7578
operator_version: main-latest
7679
use_values_override: true
77-
secrets: inherit
80+
secrets:
81+
AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }}
82+
AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }}
83+
AWS_SSH_KEY: ${{ secrets.AWS_SSH_KEY }}
84+
SLACK_BOT_TOKEN: ${{ secrets.SLACK_BOT_TOKEN }}
85+
SLACK_CHANNEL_ID: ${{ secrets.SLACK_CHANNEL_ID }}
7886

7987
notify-failure:
8088
runs-on: ubuntu-latest

tests/scripts/install-operator.sh

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -34,30 +34,26 @@ if [[ "${USE_VALUES_FILE}" == "true" ]]; then
3434
# Generate a temporary values file from environment variables
3535
# and merge it with the provided VALUES_FILE
3636
TEMP_ENV_VALUES=$(mktemp)
37-
trap 'rm -f "${TEMP_ENV_VALUES:-}" "${COMBINED_VALUES:-}"' EXIT
37+
trap 'rm -f "${TEMP_ENV_VALUES:-}"' EXIT
3838
${SCRIPT_DIR}/env-to-values.sh "${TEMP_ENV_VALUES}"
3939

40-
# If VALUES_FILE exists, merge it with env-generated values using yq
41-
# Otherwise just use the env-generated values
40+
# If VALUES_FILE exists, use both files with Helm's native multi-file
41+
# support (-f). Helm merges values in order, with later files taking
42+
# precedence — so env-generated values override the provided file.
4243
if [[ -f "${VALUES_FILE}" ]]; then
4344
echo ""
4445
echo "Using provided values file: ${VALUES_FILE}"
4546
cat "${VALUES_FILE}"
4647
echo ""
47-
echo "Merged with environment-based values:"
48+
echo "Environment-based values (takes precedence):"
4849
cat "${TEMP_ENV_VALUES}"
4950

50-
# Create a combined values file using yq for proper YAML merging
51-
COMBINED_VALUES=$(mktemp)
52-
if ! command -v yq >/dev/null 2>&1; then
53-
echo "Error: yq is required to merge YAML values files but was not found in PATH." >&2
54-
echo "Install yq: https://github.com/mikefarah/yq" >&2
55-
exit 1
56-
fi
57-
# yq merges YAML properly, with later files taking precedence
58-
yq ea '. as $item ireduce ({}; . * $item )' "${VALUES_FILE}" "${TEMP_ENV_VALUES}" > "${COMBINED_VALUES}"
59-
VALUES_FILE="${COMBINED_VALUES}"
51+
# Pass both files to helm; the env values file comes second
52+
# so its values take precedence over the override file.
53+
EXTRA_VALUES_FILES="-f ${VALUES_FILE}"
54+
VALUES_FILE="${TEMP_ENV_VALUES}"
6055
else
56+
EXTRA_VALUES_FILES=""
6157
VALUES_FILE="${TEMP_ENV_VALUES}"
6258
fi
6359

@@ -113,7 +109,9 @@ if [[ "${USE_VALUES_FILE}" == "true" ]]; then
113109
echo "Using values file approach: ${VALUES_FILE}"
114110
${HELM} install ${PROJECT_DIR}/deployments/gpu-operator --generate-name \
115111
-n "${TEST_NAMESPACE}" \
112+
${EXTRA_VALUES_FILES} \
116113
-f "${VALUES_FILE}" \
114+
${OPERATOR_OPTIONS} \
117115
--wait
118116
else
119117
echo "Using --set flags approach"

0 commit comments

Comments
 (0)