Skip to content

CP-21164: run e2e-tests since they don't run on PRs from forks - #826

Merged
FelixPhipps merged 1 commit into
masterfrom
cp-21164/fix-cyberark-validation-test-ambient-secrets
Aug 28, 2026
Merged

CP-21164: run e2e-tests since they don't run on PRs from forks#826
FelixPhipps merged 1 commit into
masterfrom
cp-21164/fix-cyberark-validation-test-ambient-secrets

Conversation

@mladen-rusev-cyberark

@mladen-rusev-cyberark mladen-rusev-cyberark commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Closes #825

Re-raises #825 from a branch in this repository rather than a fork, and carries the same
one commit (cherry-picked, original author preserved).

Why re-raise it here

The label-gated e2e jobs in .github/workflows/tests.yamlark-test-e2e (test-ark),
ngts-test-e2e (test-ngts) and test-e2e (test-e2e) — all depend on repository
secrets (GCP_SA_KEY, ARK_SUBDOMAIN/ARK_USERNAME/ARK_SECRET, ARK_OCI_BASE,
NGTS_PRIVATE_KEY). GitHub does not expose secrets to pull_request runs from forks, so
those jobs cannot pass — or even meaningfully run — on #825. Granting write access to the
fork's author wasn't an option, so the branch lives here instead.

With the three labels applied, all e2e jobs run on this PR.

What the change fixes

TestConfig_CyberArk_Validation was added alongside the Conjur JWT work and asserts on
behaviour that only holds when ARK_USERNAME/ARK_SECRET are unset. The test job in
tests.yaml sets both from real secrets at the job level (they exist for the separate
ARK_LIVE_TEST-gated live test), so those subtests inherit real credentials and fail on
master:

  • neither service_id nor ARK_USERNAME is an error at config time
  • ARK_USERNAME without ARK_SECRET is an error at config time
  • cluster_name is empty when cluster_name, cluster_id, and ARK_USERNAME are all unset

The fix clears both variables in the shared setEnv helper so the subtests are hermetic.
Subtests that need non-empty values set them after calling setEnv, so nothing is
clobbered. This is a test-isolation fix only — no production code changes.

…tion tests

CI sets both from real secrets at the job level (for the separate
ARK_LIVE_TEST-gated live test), so three subtests asserting on an
empty username/secret picked up real ambient values instead of the
empty baseline they assumed locally.
@mladen-rusev-cyberark mladen-rusev-cyberark added test-e2e To signal e2e test job to be run test-ngts test-ark labels Aug 27, 2026
@FelixPhipps

Copy link
Copy Markdown
Member

Seems logical. Approved!

@FelixPhipps
FelixPhipps merged commit 49e244a into master Aug 28, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test-ark test-e2e To signal e2e test job to be run test-ngts

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants