Allow all sst components to optionally override the DEFAULT_ACCOUNT_ID

[DJDANNY123]

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.

[vimtor]

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

[vimtor]

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

[DJDANNY123]

I don't understand your code example, I believe we can already do this with the existing implementation.
The issue is in specifying the accountId which is a required argument for all plain cloudflare components, and we can't specify the accountId in the provider.

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 },
);

[vimtor]

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

[DJDANNY123]

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.
I'm happy to update this PR when you've finished with your spree of adding new cloudflare resources

[vimtor]

// 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?

[DJDANNY123]

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.

[vimtor]

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

[DJDANNY123]

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?

[DJDANNY123]

@vimtor have you had any further thoughts on this one?

[vimtor]

let's go ahead with your approach but mark the accountId as internal and add a doc section in the cloudflare.mdx guide

[DJDANNY123]

I've updated this PR to mark each accountId as internal and I've added some documentation of supporting multiple accounts under the credentials section of the cloudflare.mdx.
Wanted to point out I've updated the public D1.get to change the function signature from a string of databaseId to an object allowing for databaseId and accountId which is a breaking change, should I look to support both and deprecate the original?

[DJDANNY123]

I've updated this with a proper deprecation of the old D1.get method to avoid breaking change, so happy for this to be reviewed now @vimtor

[DJDANNY123]

let's go ahead with your approach but mark the accountId as internal and add a doc section in the cloudflare.mdx guide

@vimtor when you have some time, could you give this a review please?