feat(core): Add ADC, filtering, and impersonation to GCP Secrets Manager - #36925
feat(core): Add ADC, filtering, and impersonation to GCP Secrets Manager#36925dzsibi wants to merge 1 commit into
Conversation
|
✅ CLA Check passed. All contributors on this PR have signed the n8n CLA — thank you! |
|
/cla-check |
There was a problem hiding this comment.
1 issue found across 7 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/cli/src/modules/external-secrets.ee/providers/gcp-secrets-manager/gcp-secrets-manager.ts">
<violation number="1" location="packages/cli/src/modules/external-secrets.ee/providers/gcp-secrets-manager/gcp-secrets-manager.ts:295">
P2: When an explicit service-account key omits `project_id` and the Project ID field is blank, this fallback can select the n8n runtime's project or fail instead of resolving the key's project, causing reads from the wrong project or no connection. Do not run ADC-style project discovery for static credentials; require the configured Project ID (or preserve and use `project_id` from the key) in that path.</violation>
</file>
Architecture diagram
sequenceDiagram
participant UI as n8n UI / Settings
participant Provider as GcpSecretsManager (Provider)
participant SDKAuth as google-auth-library
participant SDKClient as @google-cloud/secret-manager
participant GCP as Google Cloud API
Note over UI,GCP: Initialization & Connection Flow
UI->>Provider: connect(settings)
alt NEW: Use Application Default Credentials (ADC)
Provider->>SDKAuth: NEW: new GoogleAuth() (detects env credentials)
else Legacy: Service Account Key
Provider->>SDKAuth: new GoogleAuth({ credentials })
end
SDKAuth-->>Provider: sourceAuthClient
opt NEW: Impersonate Service Account
Provider->>SDKAuth: NEW: new Impersonated(sourceClient, targetPrincipal)
SDKAuth-->>Provider: impersonatedAuthClient
end
Provider->>SDKClient: NEW: new SecretManagerServiceClient({ authClient, projectId })
opt NEW: Project ID not explicitly provided
Provider->>SDKClient: auth.getProjectId()
SDKClient->>GCP: Request metadata/identity
GCP-->>SDKClient: Project ID
SDKClient-->>Provider: Project ID
end
Note over UI,GCP: Secret Retrieval Flow (List & Filter)
Provider->>SDKClient: listSecrets(request)
Note right of Provider: NEW: Includes 'filter' property if configured
SDKClient->>GCP: GET /v1/projects/{id}/secrets?filter={labelFilter}
alt Success
GCP-->>SDKClient: List of (filtered) Secrets
SDKClient-->>Provider: Secret names
else NEW: 403 / Impersonation Error
GCP-->>SDKClient: Error (e.g. Service Account Token Creator missing)
SDKClient-->>Provider: Throw Connection Error
end
loop For each Secret Name
Provider->>SDKClient: accessSecretVersion()
SDKClient->>GCP: GET /v1/projects/.../versions/latest:access
GCP-->>Provider: Secret Value
end
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| private async resolveProjectId(): Promise<void> { | ||
| if (this.settings.projectId) return; | ||
|
|
||
| const projectId = (await this.client.auth.getProjectId())?.trim(); |
There was a problem hiding this comment.
P2: When an explicit service-account key omits project_id and the Project ID field is blank, this fallback can select the n8n runtime's project or fail instead of resolving the key's project, causing reads from the wrong project or no connection. Do not run ADC-style project discovery for static credentials; require the configured Project ID (or preserve and use project_id from the key) in that path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/modules/external-secrets.ee/providers/gcp-secrets-manager/gcp-secrets-manager.ts, line 295:
<comment>When an explicit service-account key omits `project_id` and the Project ID field is blank, this fallback can select the n8n runtime's project or fail instead of resolving the key's project, causing reads from the wrong project or no connection. Do not run ADC-style project discovery for static credentials; require the configured Project ID (or preserve and use `project_id` from the key) in that path.</comment>
<file context>
@@ -218,6 +276,78 @@ export class GcpSecretsManager extends SecretsProvider {
+ private async resolveProjectId(): Promise<void> {
+ if (this.settings.projectId) return;
+
+ const projectId = (await this.client.auth.getProjectId())?.trim();
+ if (!projectId) {
+ throw new UserError(
</file context>
There was a problem hiding this comment.
I think this is a feature, not a bug. If you don't explicitly specify a project ID, we will try to default to the current project. If that fails, we return an error. This grants no additional permissions (the service account still needs access to the project), and fails loudly when testing the vault.
|
Hey @dzsibi, Thank you for your contribution. We appreciate the time and effort you’ve taken to submit this pull request. Before we can proceed, please ensure the following: • Your PR references the GitHub issue it fixes (or, for feature requests, a link to the corresponding community forum post). • Tests are included for any new functionality, logic changes or bug fixes. • The PR aligns with our contribution guidelines. Why the linked issue matters: Our teams pick up work from the issue, not from individual pull requests — the issue is what reaches them, with your PR linked to it. So please make sure the issue contains everything needed to judge the change: a clear problem description, reproduction steps, and the expected behaviour. If the issue is thin, add the missing context there rather than only in the PR description. Regarding new nodes: We no longer accept new nodes directly into the core codebase. Instead, we encourage contributors to follow our Community Node Submission Guide to publish nodes independently. If your node integrates with an AI service that you own or represent, please email nodes@n8n.io and we will be happy to discuss the best approach. About review timelines: While we plan to review it as soon as possible, we are currently unable to provide an exact timeframe. Our goal is to begin reviews within a month, but this may change depending on team priorities. We will reach out when the review begins. Please also note that other contributors may have opened pull requests for the same issue. We keep them all open so the reviewing team can choose the approach that fits best. Once the issue is resolved, the remaining pull requests are closed — this is not a judgement on the quality of your work, and we're grateful for it either way. Thank you again for contributing to n8n. |
Summary
The PR adds functionality required to use the GCP Secret Manager integration from inside an environment like GKE or Cloud Run, where the service account associated with container can be used to access secrets, without injecting explicit service account credentials. This is achieved by relying on the Google SDK's Application Default Credentials flow, which detects credentials in the environment automatically.
The PR includes other, supporting changes, without which this functionality is not complete. These are:
These are needed to control the contents of the individual vaults. Previously, it was a valid strategy to create a separate service account for each vault, only grant access to it for the secrets we want to load into that vault, then n8n would ignore any secrets it was unable to fetch, even if it could list them. By adding these options, one could now continue using separate service accounts, but without explicitly injecting their credentials (via impersonation), or one could simply add a label filter, which will do server-side filtering and limit which secrets are loaded into each vault, even if the default service account has permissions to read all secrets. Overriding the project ID also lets one fetch secrets from a different project, even if using the service account in the current project.
I created this PR to unblock our internal deployment of n8n, and to address the security and compliance concerns that stem from hard coded credentials and the requirement to create and export new credentials for each new vault we create. I have found traces of other users requesting the same (see linked forum posts), so while I can continue to maintain this in our fork, it would be much better to merge this upstream and share it with the community. I am open to any feedback on my implementation.
Note: the PR includes the removal of
google-gax's version pin, and an update to the two Google libraries that use this. This was required to address a version conflict between thegoogle-gaxpeer dependencies ofsecret-managerandresource-manager, which should now be resolved.How to test
n8n-vault=financeto only one secret.Secret Manager Secret AccessorandSecret Manager Secret Viewerroles.GOOGLE_APPLICATION_CREDENTIALS. On GCP, use an attached runtime identity.labels.n8n-vault=financeand reload the vault. Confirm that only the labeled secret is available.Automated tests / linting was run locally. I have also deployed the changes to our internal environments, and are now using this with multiple vaults successfully.
Related Linear tickets, Github issues, and Community forum posts
Review / Merge checklist
Backport to Beta,Backport to Stable, orBackport to v1(if the PR is an urgent fix that needs to be backported)