Skip to content

Simplify aws policy by adding cortex prefix to queue name #1941

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Mar 11, 2021
Merged

Simplify aws policy by adding cortex prefix to queue name #1941

merged 3 commits into from
Mar 11, 2021

Conversation

@vishalbollu
Copy link
Contributor

@vishalbollu vishalbollu commented Mar 5, 2021 •
edited
Loading

checklist:

  • run make test and make lint
  • test manually (i.e. build/push all images, restart operator, and re-deploy APIs)

miguelvr
miguelvr reviewed Mar 9, 2021
View reviewed changes
docs/clusters/aws/auth.md
"Effect": "Allow",
"Action": "sqs:*",
"Resource": "arn:aws:sqs:{{ .Region }}:{{ .AccountID }}:{{ .SQSPrefix }}*"
"Resource": "arn:aws:sqs:{{ .Region }}:{{ .AccountID }}:cortex-*"
Copy link
Collaborator

@miguelvr miguelvr Mar 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: convert to const

Copy link
Contributor Author

@vishalbollu vishalbollu Mar 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean here?

Copy link
Collaborator

@miguelvr miguelvr Mar 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cortex- prefix

miguelvr
miguelvr reviewed Mar 9, 2021
View reviewed changes
miguelvr
miguelvr reviewed Mar 9, 2021
View reviewed changes
pkg/types/clusterconfig/aws_policy.go
"Effect": "Allow",
"Action": "sqs:*",
"Resource": "arn:aws:sqs:{{ .Region }}:{{ .AccountID }}:{{ .SQSPrefix }}*"
"Resource": "arn:aws:sqs:{{ .Region }}:{{ .AccountID }}:cortex-*"
Copy link
Collaborator

@miguelvr miguelvr Mar 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: convert to const

miguelvr
miguelvr reviewed Mar 9, 2021
View reviewed changes
pkg/types/clusterconfig/cluster_config_aws.go
// 10 was chosen to make sure that other identifiers can be added to the full queue name before reaching the 80 char SQS name limit
return hash.String(clusterName)[:10] + "-"
// 8 was chosen to make sure that other identifiers can be added to the full queue name before reaching the 80 char SQS name limit
return "cortex-" + hash.String(clusterName)[:8] + "-"
Copy link
Collaborator

@miguelvr miguelvr Mar 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: convert to const

@RobertLucian RobertLucian changed the title Simplify aws policy by add cortex prefix to queue name Simplify aws policy by adding cortex prefix to queue name Mar 9, 2021
@vishalbollu vishalbollu merged commit df2c91d into master Mar 11, 2021
@vishalbollu vishalbollu deleted the remove-sqs-prefix-from-policy branch March 11, 2021 18:56
@vishalbollu vishalbollu added this to the 0.31 milestone Mar 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@miguelvr miguelvr miguelvr left review comments

@deliahu deliahu deliahu approved these changes

@RobertLucian RobertLucian RobertLucian approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v0.31

Development

Successfully merging this pull request may close these issues.

5 participants

@vishalbollu @deliahu @miguelvr @RobertLucian