feat: run PR acceptance tests with OrgManager role in shared CF org - #1149
Open
bonzofenix wants to merge 125 commits into
Open
feat: run PR acceptance tests with OrgManager role in shared CF org#1149bonzofenix wants to merge 125 commits into
bonzofenix wants to merge 125 commits into
Conversation
bonzofenix
force-pushed
the
fixed-acceptance-test-org-v2
branch
from
April 13, 2026 19:33
1f2b0ad to
a7210a4
Compare
silvestre
marked this pull request as draft
April 14, 2026 13:25
bonzofenix
force-pushed
the
fixed-acceptance-test-org-v2
branch
from
April 14, 2026 14:28
d9d9381 to
6fc8f35
Compare
bonzofenix
marked this pull request as ready for review
April 14, 2026 20:52
silvestre
self-requested a review
April 15, 2026 07:17
bonzofenix
force-pushed
the
fixed-acceptance-test-org-v2
branch
from
April 20, 2026 23:02
a9a6cdc to
158189d
Compare
|
bonzofenix
force-pushed
the
fixed-acceptance-test-org-v2
branch
from
June 3, 2026 09:38
2d02df7 to
903a71d
Compare
bonzofenix
force-pushed
the
fixed-acceptance-test-org-v2
branch
2 times, most recently
from
June 15, 2026 16:11
875dfd9 to
9b9550d
Compare
bonzofenix
marked this pull request as draft
June 17, 2026 07:36
This was referenced Jun 17, 2026
bonzofenix
force-pushed
the
fixed-acceptance-test-org-v2
branch
2 times, most recently
from
June 25, 2026 17:37
95fb9d4 to
b7897b6
Compare
The go-log-cache library's OAuth2 client sends credentials in the request body, but CF's "cf" UAA client requires Basic auth header. This adds a custom CFOauth2HTTPClient that: - Sends client_id:client_secret via Basic auth header - Uses password grant with username/password in form body - Handles 401 responses with automatic token refresh - Prevents race conditions with mutex-protected token storage - Supports token expiration with configurable buffer This enables the Event Generator to authenticate as org manager users when fetching metrics from Log Cache, instead of requiring dedicated UAA clients with client_credentials. Files: - eventgenerator/metric/cf_oauth2_client.go: Custom OAuth2 HTTP client - eventgenerator/metric/cf_oauth2_client_test.go: Comprehensive tests - eventgenerator/metric/fetcher_factory.go: Password grant detection - eventgenerator/metric/fetcher_factory_test.go: Factory tests - models/uaa_creds.go: GrantType/Username/Password fields
Enable running acceptance tests with separate org manager users instead of requiring full admin privileges. This includes: Acceptance test configuration: - Support for existing org/space/user reuse - Skip service access management option - Per-PR test user isolation via AUTOSCALER_TEST_USER New scripts: - setup-org-manager-user.sh: Create/configure org manager test users - org-manager-login.sh: Login as org manager - enable-service-access.sh: Enable service access as admin - set-security-group.sh: Configure security groups for autoscaler - register-broker.sh: Register service broker with proper permissions CI workflow changes: - Add cleanup, user setup, and security group steps before deployment - Use register-broker target instead of deploy-register-cf Acceptance test helpers: - Org creation/management helpers - Cleanup improvements for test isolation - Config support for OrgManager-level permissions
Extend the CF client to support OAuth2 password grant in addition to client_credentials. This enables authentication using org manager user credentials for components that need CF API access. Changes: - cf/config.go: Add GrantType, Username, Password fields with validation - cf/client.go: Add grant type constants - cf/cfclient_wrapper.go: Use UserPassword config for password grant, switch introspect to use Basic auth directly - cf/config_test.go: Add password grant validation tests - cf/cfclient_wrapper_test.go: Add password grant client creation test
The gosec linter no longer flags these lines, making the //nolint:gosec directives unnecessary. Removing them fixes nolintlint errors in CI.
- Add #nosec G704 directives for HTTP client Do() calls in cfclient_wrapper.go and cf_oauth2_client.go (URLs come from trusted CF API endpoints or user configuration) - Fix config deserialization test to initialize a fresh Config before unmarshaling, preventing state leakage from password grant validation tests
…mments Use rejected-token comparison in forceRefreshToken() so that when multiple goroutines receive 401s simultaneously, only the first one to acquire the lock refreshes. Others see the token has changed and reuse the new one. Remove comments that restate what the code does.
…pecs The cf_oauth2_client_test.go used package metric (white-box) with its own TestCFOauth2HTTPClient/RunSpecs entry point. This caused Ginkgo to fail when running go test because TestMetricsSuite also calls RunSpecs in the same process — Ginkgo does not support rerunning suites. Convert to package metric_test so all specs run under the single TestMetricsSuite runner. Replace internal field assertions with behavioral tests that verify URL handling via actual HTTP requests.
- Add return 0 to all functions missing explicit returns - Replace [ with [[ for safer conditional tests - Redirect error messages to stderr with >&2 - Extract separator constant in enable-service-access.sh Fixes 17 MAJOR shell issues flagged by SonarCloud
Conditionally use different CF users based on branch: - Main branch: admin user (full privileges) - PR branches: org-manager user (isolated per-PR) Changes: - CI: Skip org-manager setup on main branch - Config: Detect PR_NUMBER to choose credentials - Scripts: Introduce cf_deployment_login for context-aware auth - Scripts: Keep cf_admin_login for truly admin-only ops Benefits: - Main keeps using admin (backward compatible) - PRs use isolated users (better security, parallel testing) - Clear separation: cf_admin_login vs cf_deployment_login
- Move GetSpaceGuid() call outside loop to avoid redundant API calls - Enhance BoolOrInt comment explaining CF API behavior difference between admin and org-manager users
- Add AUTOSCALER_ORG_MANAGER_PASSWORD validation before component credential export - Add AUTOSCALER_ORG_MANAGER_USER validation in cf_deployment_login - Replace silent error suppression in cleanup_apps with proper error discrimination - Log warnings for cf set-space-role and cf create-space failures - Switch eventgenerator to password grant via EVENTGENERATOR_CF_* vars - Remove unused EVENTGENERATOR_LOG_CACHE_UAA_* secret loading - Fix acceptance Makefile echo (undefined variable → literal path) - Change pipeline.yml branch back to main for post-merge - Add security comments on printenv secret filter regex
…tions - Add is_pr_deployment()/is_main_deployment() helpers to common.sh - Replace all inline PR_NUMBER checks with helper functions - Make password grant conditional on PR deployments only - Main deployments use client_credentials with autoscaler_client_id - Add secret/client_secret fields to template for client_credentials - Fix cf_target() comment wording per reviewer suggestion
The cleanup-autoscaler-deployments script lists all orgs containing 'autoscaler' and deletes them. This bypasses the guard in common.sh (AUTOSCALER_ORG != DEPLOYMENT_NAME) because the script sets DEPLOYMENT_NAME to the org name itself, making both vars equal. Add a PROTECTED_ORGS list and filter them out of get_autoscaler_deployments() so shared orgs are never targeted by the scheduled cleanup.
…lling - cleanup-autoscaler-deployments.sh: replace dynamic jq filter loop with is_protected_org() helper and pipe filter — clearer and extensible - run-mta-acceptance-tests.sh: query get_pr_tasks() once per poll cycle and pass result to both has_unfinished_tasks() and show_status(), halving CF API calls during monitoring
…t on setup - mta-deploy.sh: replace set +e block with explicit error checking for cf curl v3/service_brokers — failures now exit with clear message instead of silently producing empty variable - create-org-manager-user.sh: ensure org and space exist before targeting, so first-time main deployments don't fail when space is missing
- acceptance-tests-config.sh: validate AUTOSCALER_ORG_MANAGER_PASSWORD before use in PR mode (matches build-extension-file.sh) - build-extension-file.sh: validate AUTOSCALER_OTHER_USER_PASSWORD for acceptance test credentials - common.sh: fix stale comment about cf_target creating spaces
Script now creates org/space and test users — broader than just user creation. Also deduplicate job-level AUTOSCALER_ORG env in reusable workflow (already set at workflow level).
bonzofenix
force-pushed
the
fixed-acceptance-test-org-v2
branch
from
June 29, 2026 08:06
9376ae4 to
e157179
Compare
The public "cf" OAuth client (used for PR password-grant deployments) cannot authenticate against UAA /introspect, which requires a confidential client. This caused every policy API call to return 500 "Failed to check if user is admin". On password-grant deployments the caller is an OrgManager user, never a CC admin, so returning false directly is semantically correct. The middleware falls through to IsUserSpaceDeveloper which authorizes SpaceDevelopers correctly.
…allback Drop the redundant existing_organization workflow input — build-extension-file.sh already defaults EXISTING_ORGANIZATION to AUTOSCALER_ORG internally. Also fix a latent bug where non-PR runs passed AUTOSCALER_ORG=system, overriding the vars.source.sh default of DEPLOYMENT_NAME. Now passes empty string so the script default takes effect correctly.
Contributor
|
closing in favour of #1149 |
…s to acceptance job only
- Add INFRASTRUCTURE var (default OSS) to vars.source.sh - Add is_oss_infrastructure() predicate to common.sh - Extract create-acceptance-space.sh (runs before provision-db) - Add provision-db-service.sh for non-OSS: cf create-service using POSTGRES_SERVICE_OFFERING/PLAN vars instead of bosh/credhub - build-extension-file.sh: skip bbl_login and credhub on non-OSS; patch database resource to org.cloudfoundry.existing-service post-envsubst - acceptance_tests_reusable.yaml: add infrastructure/postgres inputs, conditional provision-db vs provision-db-service steps
The create-acceptance-space step was missing AUTOSCALER_ORG_MANAGER_USER and AUTOSCALER_ORG_MANAGER_PASSWORD env vars. The cf_deployment_login() function in common.sh requires both for non-main (PR) deployments, causing the deploy job to fail with "ERROR: AUTOSCALER_ORG_MANAGER_PASSWORD is not set".
- cleanup-autoscaler.sh: skip bbl_login/credhub/deprovision_db on non-OSS; use cf_deployment_login; delete CF service instance instead - acceptance_tests_reusable.yaml: pass INFRASTRUCTURE + CF credentials to the cleanup step
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What and why
PR CI pipelines required admin credentials to deploy and run acceptance tests, blocking use in non-admin environments. This PR removes that dependency: PRs deploy into
SAP_autoscaler_tests_OSSusing OrgManager credentials; main keeps admin unchanged.Changes
cf_deployment_login— picks admin (main) or OrgManager (PR) based onPR_NUMBER. Fails fast with a clear error ifAUTOSCALER_ORG_MANAGER_PASSWORDis unset. All deploy/test/cleanup scripts use this instead of hardcodedcf_login.Password grant for components —
eventgenerator,scalingengine,operatorusegrant_type: passwordwith OrgManager credentials on PRs. Replacesclient_credentialswhich required admin. Extension template wired accordingly.Space-scoped broker — PRs register the service broker
--space-scoped(OrgManager-compatible). Main uses global registration unchanged.cf_targetsimplified —find_or_create_{org,space}removed; org and space are pre-provisioned by the workflow.cf_targetis now a plaincf target -o … -s ….Space creation owned by workflow — the "Create space and assign SpaceDeveloper" step (PR-only, gated with
if: pull_request) creates the space as org-manager-user so it retains the creator's SpaceDeveloper role. The test runner re-grants SpaceDeveloper at startup to cover the race where another workflow recreates the space between jobs.org-manager-user never deleted —
cleanup-acceptance.shand its Makefile targets removed.cleanup_test_userremoved fromcommon.sh. The shared org-manager user is managed out-of-band by the one-offcreate-org-manager-userworkflow.Security groups moved to infra pipeline — postgres and metricsgateway security groups bound org-wide once by
deploy-postgres.sh/deploy-metricsgateway.sh. Per-PR security group setup removed frommetricsforwarder/Makefileandmetricsgateway/Makefile.Shared metricsgateway — PRs point metricsforwarder at
system-production-metricsgatewayviaMETRICSFORWARDER_METRICS_GATEWAY_URLrepo variable, fixing XFCC validation (cert org guid must match the gateway's org).BoolOrIntin broker tests — CF returnsplan_updateableasboolfor admin,intfor OrgManager. Unmarshaler now tries bool first, then int.Credential safety —
printenv >> $GITHUB_ENVreplaced withprintenv | grep -vE '^(AUTOSCALER_ORG_MANAGER_PASSWORD|AUTOSCALER_OTHER_USER_PASSWORD|GH_TOKEN)='across all three jobs in the reusable workflow.Test plan
SAP_autoscaler_tests_OSSorgsystem-production-metricsgateway🤖 Updated with Claude Code