Allow all sst components to optionally override the DEFAULT_ACCOUNT_ID#6770
Allow all sst components to optionally override the DEFAULT_ACCOUNT_ID#6770DJDANNY123 wants to merge 1 commit intoanomalyco:devfrom
Conversation
|
thanks for your contribution @DJDANNY123 this week we're adding a lot of Cloudflare components, let's wait until we finish so you can update your PR in the meantime, i wonder if this is the ideal setup. maybe we should let you define multiple providers, each using a different account. not sure how that would look, though |
|
just realized that what we should do is to find a way to do this: const provider = new cloudflare.Provider("OtherAccount", {
apiToken: "",
})
new sst.cloudflare.Bucket("Bucket", {}, { provider });this would match the aws behaviour |
|
I don't understand your code example, I believe we can already do this with the existing implementation. Ideally the cloudflare provider would accept an accountId, and use this for relevant resources but this would probably be a fairly major change in the cloudflare Pulumi resource provider. I think given the current state of the downstream provider, we should mimic how the downstream provider works (but still keep the existing default account behaviour for ease of people who don't need to do multi-account deployments) // define a provider with another api token for a different account
const provider = new cloudflare.Provider("AnotherProvider", { apiToken: "" });
// deploy a plain cloudflare resource in the other account
const plainCfResource = new cloudflare.R2Bucket(
"",
{ name: "MyBucket", accountId: "ANOTHER_ACCOUNT_ID" },
{ provider },
);
// deploy a sst component in another account in the same way
const sstResource = new sst.cloudflare.Worker(
"MyWorker",
{ handler: "index.ts", accountId: "ANOTHER_ACCOUNT_ID" },
{ provider },
); |
|
yeah, we should keep the default account behavior what i'm saying is that the code snippet you provided might already work, so i'm not sure if this pr is necessary |
|
The snippet I provided would work if every SST component had an optional accountId as part of their arguments, and was applied to all resources provisioned by these components, however this isn't the case currently. |
// define a provider with another api token for a different account
const provider = new cloudflare.Provider("AnotherProvider", { apiToken: "" });
// deploy a plain cloudflare resource in the other account
const plainCfResource = new cloudflare.R2Bucket(
"",
{ name: "MyBucket" },
{ provider },
);
// deploy a sst component in another account in the same way
const sstResource = new sst.cloudflare.Worker(
"MyWorker",
{ handler: "index.ts" },
{ provider },
);we should strive for this and push the accountId selection logic inside each component does that sound good? |
|
The only way I see to consistently do this is an update to the main cloudflare provider to accept accountId as a provider option. If we try to inject vanilla cloudflare pulumi constructs with account IDs I feel like it would get messy and deviate from SST being a simple wrapper around plain pulumi, as the types of arguments for these plain constructs would also have to change, but I'm open to hear if there's a way to achieve this I haven't considered. |
|
i guess we could do something like: import * as cloudflare from "@pulumi/cloudflare";
const account = cloudflare.getAccountsOutput({}).accounts.apply(accounts => {
if (accounts.length !== 1) {
throw new Error(`expected exactly one Cloudflare account, got ${accounts.length}`);
}
return accounts[0];
});
export const accountId = account.id;and force people's tokens to be exactly for one account not sure, let me think about it more |
|
I like the idea, I just don't know if it's much better than the alternative of optionally supplying the account id, as the improved ergonomics may not outweigh the inconsistency of specifying account id, as with the vanilla cloudflare pulumi components, accountId is a required variable. Unless we were able to modify this as well by overriding all of these vanilla components to omit this from the argument types? |
Closes #6769
Context
The Cloudflare provider is a bit weird in the way it expects you to specify the accountId on the resource, but API token on the provider.
SST made the choice to default the accountId to an environment variable for all the resources it provisions, as it is annoying to specify this for every resource, however this makes it difficult for multi-account deployments.
This is compounded as some of the resources can't be accessed/overridden by the transforms property (e.g. the custom domain of a worker).
This Change
This change simply extends each cloudflare SST component with an optional accountId, which will be used if specified for provisioning/querying all resources within this component.