feat(cli): fall back to Application Default Credentials for secrets commands#898
Conversation
…ommands Co-Authored-By: AJ Steers <aj@airbyte.io>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. 💡 Show Tips and TricksTesting This CDK VersionYou can test this version of the CDK using the following: # Run the CLI from this branch:
uvx 'git+https://github.com/airbytehq/airbyte-python-cdk.git@devin/1770336012-adc-fallback-secrets#egg=airbyte-python-cdk[dev]' --help
# Update a connector to use the CDK from this branch ref:
cd airbyte-integrations/connectors/source-example
poe use-cdk-branch devin/1770336012-adc-fallback-secretsPR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
|
There was a problem hiding this comment.
Pull request overview
This PR enables the airbyte-cdk secrets CLI commands to use Google Cloud's Application Default Credentials (ADC) as a fallback when the GCP_GSM_CREDENTIALS environment variable is not set, removing the previous hard requirement for service account JSON credentials.
Changes:
- Modified
_get_gsm_secrets_client()to makeGCP_GSM_CREDENTIALSoptional - Added fallback to ADC when environment variable is not set
- Enhanced function docstring to document the new authentication flow
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
|
Devin, lint failed. |
Co-Authored-By: AJ Steers <aj@airbyte.io>
📝 WalkthroughWalkthroughAuthentication for the GSM secrets client now prefers Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant SecretsFn as _get_gsm_secrets_client
participant GSMClient as SecretManagerServiceClient
participant Auth as GoogleAuth/ADC
Caller->>SecretsFn: request GSM client
SecretsFn->>SecretsFn: read GCP_GSM_CREDENTIALS env
alt GCP_GSM_CREDENTIALS set
SecretsFn->>GSMClient: from_service_account_info(credentials_json)
GSMClient-->>SecretsFn: client instance
else GCP_GSM_CREDENTIALS unset
SecretsFn->>GSMClient: SecretManagerServiceClient() (ADC)
GSMClient->>Auth: resolve ADC
alt ADC available
Auth-->>GSMClient: credentials
GSMClient-->>SecretsFn: client instance
else ADC missing -> DefaultCredentialsError
GSMClient-->>SecretsFn: raises DefaultCredentialsError
SecretsFn-->>Caller: raise ValueError (advise set env or run gcloud auth)
end
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@airbyte_cdk/cli/airbyte_cdk/_secrets.py`:
- Around line 441-444: Remove the redundant typing.cast wrapping the
SecretManagerServiceClient instantiation: replace the return of return
cast("secretmanager.SecretManagerServiceClient",
secretmanager.SecretManagerServiceClient()) with a direct return
secretmanager.SecretManagerServiceClient(); also remove the now-unused cast
import if it becomes unused by other code. This targets the return cast(...)
expression that constructs secretmanager.SecretManagerServiceClient.
🧹 Nitpick comments (1)
airbyte_cdk/cli/airbyte_cdk/_secrets.py (1)
432-444: Consider a user-friendly hint when ADC also fails.Previously, a missing
GCP_GSM_CREDENTIALSraised aValueErrorwith a clear message telling the user what to do. Now, if neither the env var nor ADC is configured, the user will get a rawgoogle.auth.exceptions.DefaultCredentialsError, which can be cryptic.Would it be worth wrapping the ADC client construction in a try/except to catch
DefaultCredentialsErrorand re-raise with a friendlier message — something like "Set GCP_GSM_CREDENTIALS or rungcloud auth application-default login"? Totally optional, but it could save folks some head-scratching, wdyt?💡 Possible approach
+ try: + return secretmanager.SecretManagerServiceClient() + except Exception as ex: + raise ValueError( + "GCP credentials not found. Either set the GCP_GSM_CREDENTIALS " + "environment variable with service account JSON, or run " + "'gcloud auth application-default login' to configure Application " + "Default Credentials." + ) from ex - return cast( - "secretmanager.SecretManagerServiceClient", - secretmanager.SecretManagerServiceClient(), - )
Co-Authored-By: AJ Steers <aj@airbyte.io>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@airbyte_cdk/cli/airbyte_cdk/_secrets.py`:
- Line 39: Move the top-level import of google.auth.exceptions into the existing
guarded try/except that imports google.cloud.secretmanager_v1 so the module can
import even when google-auth isn't installed; specifically, modify the try block
that defines secretmanager to also import google.auth.exceptions (and set a
fallback None in the except ImportError) so references inside
_get_gsm_secrets_client and the early if not secretmanager guard remain safe and
the helpful runtime error is preserved.
…ceful degradation Co-Authored-By: AJ Steers <aj@airbyte.io>
…s used Co-Authored-By: AJ Steers <aj@airbyte.io>
|
❤️ Works now even without the |
Aaron ("AJ") Steers (aaronsteers)
deleted the
devin/1770336012-adc-fallback-secrets
branch
Summary
Previously,
airbyte-cdk secrets listandairbyte-cdk secrets fetchrequired theGCP_GSM_CREDENTIALSenvironment variable (service account JSON) and raised aValueErrorif it was missing. This change makes the env var optional by falling back to Application Default Credentials (ADC) when it's not set.This means engineers can use
gcloud auth application-default loginto authenticate, without needing to export a service account key.When
GCP_GSM_CREDENTIALSis set, behavior is unchanged (service account credentials are used).Review & Testing Checklist for Human
GCP_GSM_CREDENTIALSnor ADC credentials are available, the GCP client will raisegoogle.auth.exceptions.DefaultCredentialsErrorinstead of the previousValueErrorwith a specific message. Confirm this error is clear enough, or decide if a try/except wrapper with a friendlier message is warranted.gcloud auth application-default login, unsetGCP_GSM_CREDENTIALS, and verifyairbyte-cdk secrets list source-pokeapiworks with your user credentials.GCP_GSM_CREDENTIALSset, verifysecrets list/secrets fetchstill work as before.Notes
Summary by CodeRabbit
Bug Fixes
Documentation
Important
Auto-merge enabled.
This PR is set to merge automatically when all requirements are met.
Note
Auto-merge may have been disabled. Please check the PR status to confirm.