Skip to content

Conversation

@lantoli
Copy link
Member

@lantoli lantoli commented Oct 17, 2025

Description

Adjust migration tests with SA and access token

  • Migration tests can't be enabled as is with SA because each migration test will create an SA token causing token creation limit issues, execution example
  • Instead dedicated jobs for migration tests are used, e.g. config_sa_mig (execution example) or advanced_cluster_sa_mig (execution example).
  • Remove SA smoke tests as we already run SA in Test Suite in alternate days

Link to any related issue(s): CLOUDP-352095

Type of change:

  • Bug fix (non-breaking change which fixes an issue). Please, add the "bug" label to the PR.
  • New feature (non-breaking change which adds functionality). Please, add the "enhancement" label to the PR. A migration guide must be created or updated if the new feature will go in a major version.
  • Breaking change (fix or feature that would cause existing functionality to not work as expected). Please, add the "breaking change" label to the PR. A migration guide must be created or updated.
  • This change requires a documentation update
  • Documentation fix/enhancement

Required Checklist:

  • I have signed the MongoDB CLA
  • I have read the contributing guides
  • I have checked that this change does not generate any credentials and that they are NOT accidentally logged anywhere.
  • I have added tests that prove my fix is effective or that my feature works per HashiCorp requirements
  • I have added any necessary documentation (if appropriate)
  • I have run make fmt and formatted my code
  • If changes include deprecations or removals I have added appropriate changelog entries.
  • If changes include removal or addition of 3rd party GitHub actions, I updated our internal document. Reach out to the APIx Integration slack channel to get access to the internal document.

Further comments

* master:
  chore: Update SSDLC report for v2.1.0
  chore: Updates CHANGELOG.md header for v2.1.0 release
  chore: Update example links in registry docs for v2.1.0 release
  chore: Add support for typed nested objects in autogen (#3801)
  chore: Update examples and doc to use SA instead of PAK (#3804)
  chore: Updates CHANGELOG.md for #3793
  feat: Add workspace_name field in stream_processor resource and datasource (#3793)
  doc: Adds examples for advanced_cluster upgrade to v2.0 & improves documentation (#3767)
  chore: Fix tests using GetIndependentShardScalingMode (#3799)
  chore: Don't send notifications in PRs or release (#3797)
  test: Adds delay to avoid flakiness in TestAccProjectAPIKey_recreateWhenDeletedExternally (#3798)
  move CloseTokenSource call out of defer (#3796)
  chore: Revoke SA tokens after Terraform command finishes (#3794)
  chore: Fix send_notification in Test Suite (#3792)
  chore: Document Troubleshooting SA (#3791)
  build(deps): bump go.mongodb.org/atlas-sdk (#3776)
@lantoli lantoli changed the title chore: Enable migration tests with SA chore: Adjust migration tests with SA Oct 23, 2025
* master:
  chore: Invoke Delete operation if timeout in autogen (#3820)
  Add support for typed nested sets in autogen (#3819)
  chore: Use timeouts attributes in autogen resources (#3817)
  refactor: Adds invalid update error for delete_on_create_timeout for sdkv2 (#3810)
  Add support for typed lists in autogen (#3816)
  refactor: Uses delete_on_create_timeout with default=true support across TPF resources (#3809)
  Rename customtype package to customtypes (#3814)
  chore: Add support for typed nested lists in autogen (#3813)
  don't schedule matrix until CLOUDP-353513 (#3812)
  chore: Fix stream tests notifications (#3805)
  chore: Bump golang.org/x/oauth2 from 0.31.0 to 0.32.0 (#3803)
  depends on data sources (#3808)
  test: Avoids flaky test by simplifying the check, ensuring count > 0 instead of reading API response and trying exact match (#3806)
  chore: Generate timeouts attribute for autogen resources (#3802)

# Conflicts:
#	.github/workflows/acceptance-tests-runner.yml
@lantoli lantoli changed the title chore: Adjust migration tests with SA chore: Adjust migration tests with SA and access token Oct 29, 2025
@lantoli lantoli marked this pull request as ready for review October 29, 2025 08:39
@lantoli lantoli requested a review from a team as a code owner October 29, 2025 08:39
Copilot AI review requested due to automatic review settings October 29, 2025 08:39
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adjusts migration tests to work with Service Account (SA) authentication by introducing access token support. The change prevents token creation limit issues that occur when each migration test creates its own SA token. Migration tests using SA now run in dedicated GitHub Actions jobs using pre-created access tokens instead of SA credentials directly.

Key changes:

  • Replaced generate-oauth2-token tool with enhanced access-token tool that supports both token creation and revocation
  • Added access token authentication support alongside existing PAK and SA credential types
  • Created dedicated GitHub Actions jobs for SA migration tests that use access tokens

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tools/generate-oauth2-token/main.go Removed old OAuth2 token generation tool
tools/access-token/main.go Added new tool with create/revoke commands and GitHub Actions integration
internal/testutil/acc/skip.go Added access token detection and skip helper function
internal/testutil/acc/pre_check.go Updated credential validation to include access token as valid auth method
internal/testutil/acc/factory.go Added access token to provider configuration
internal/service/project/resource_project_test.go Added access token skip for Gov test
internal/service/project/resource_project_migration_test.go Added access token skip for Gov migration test
internal/provider/provider_authentication_test.go Added access token skips for credential-specific tests
internal/config/service_account.go Added TODO comment (temporary marker)
Makefile Updated make targets to use new access-token tool commands
.github/workflows/code-health.yml Removed needs dependency from acceptance tests workflow
.github/workflows/acceptance-tests-runner.yml Added dedicated SA migration test jobs and token lifecycle management

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Collaborator

@manupedrozo manupedrozo left a comment

Choose a reason for hiding this comment

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

Nice, I really like this workaround.

Comment on lines +504 to +507
MONGODB_ATLAS_PUBLIC_KEY: ""
MONGODB_ATLAS_PRIVATE_KEY: ""
MONGODB_ATLAS_CLIENT_ID: ""
MONGODB_ATLAS_CLIENT_SECRET: ""
Copy link
Collaborator

Choose a reason for hiding this comment

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

Need to explicitly set these to empty string?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes because they're set here in the global workflow environment, so needed to override them:

MONGODB_ATLAS_ORG_ID: ${{ inputs.mongodb_atlas_org_id }}

and in tests we verify that they is only one credentials mechanism to avoid issues:

Copy link
Collaborator

Choose a reason for hiding this comment

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

Got it, ty!

@lantoli lantoli merged commit 20fbbf2 into master Oct 29, 2025
50 checks passed
@lantoli lantoli deleted the CLOUDP-352095_sa_mig_tests branch October 29, 2025 11:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants