Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot set snsTopicArn to disabled in fxa-graphql-api #17007

Closed
jackyzy823 opened this issue May 24, 2024 · 1 comment
Closed

Cannot set snsTopicArn to disabled in fxa-graphql-api #17007

jackyzy823 opened this issue May 24, 2024 · 1 comment

Comments

@jackyzy823
Copy link
Contributor

jackyzy823 commented May 24, 2024

Description

It is said

doc: 'Amazon SNS topic on which to send account event notifications. Set to "disabled" to turn off the notifier',

But actually:

export function setupSns(config: NotifierSnsConfig) {
const endpoint = config.snsTopicEndpoint;
const region = config.snsTopicArn.split(':')[3];
return new SNS({
region,
endpoint,
});
}

I think this is copied from

doc: 'Amazon SNS topic on which to send account event notifications. Set to "disabled" to turn off the notifier',

but it's implementation has consider the disabled condition.

if (notifierSnsTopicArn !== 'disabled') {
// Pull the region info out of the topic arn.
// For some reason we need to pass this in explicitly.
// Format is "arn:aws:sns:<region>:<other junk>"
const region = notifierSnsTopicArn.split(':')[3];
// This will pull in default credentials, region data etc
// from the metadata service available to the instance.
// It's magic, and it's awesome.
sns = new AWS.SNS({ endpoint: notifierSnsTopicEndpoint, region: region });
}


Also when snsTopicArn is disabled, error shouldn't be raised when snsTopicEndpoint is undefined

if (!config.snsTopicEndpoint) {
throw new Error('Config error notifierSnsTopicEndpoint missing');
}

┆Issue is synchronized with this Jira Task

@vpomerleau
Copy link
Contributor

Thank you for the report! We discussed this issue during triage and have moved it into our Jira backlog. PRs for this are welcome, but it's not currently on our short-term roadmap.

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

No branches or pull requests

2 participants