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

Merged
merged 3 commits into from
Mar 11, 2021
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
35e504a
Simplify aws policy by changing sqs queue name
vishalbollu Mar 5, 2021
18066ce
Remove print statement
vishalbollu Mar 10, 2021
60ec4fc
Merge branch 'master' into remove-sqs-prefix-from-policy
vishalbollu Mar 11, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion cli/cmd/cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,6 @@ var _clusterUpCmd = &cobra.Command{
LogGroup: clusterConfig.ClusterName,
Bucket: clusterConfig.Bucket,
Region: clusterConfig.Region,
SQSPrefix: clusterconfig.SQSNamePrefix(clusterConfig.ClusterName),
AccountID: accountID,
})
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion docs/clusters/aws/auth.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ _NOTE: The policy created during `cortex cluster up` will automatically be delet
{
"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

},
{
"Effect": "Allow",
Expand Down
4 changes: 2 additions & 2 deletions pkg/operator/resources/job/batchapi/queue.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func apiQueueNamePrefix(apiName string) string {
return config.CoreConfig.SQSNamePrefix() + apiName + "-"
}

// QueueName is <hash of cluster name>-<api_name>-<job_id>.fifo
// QueueName is cortex-<hash of cluster name>-<api_name>-<job_id>.fifo
func getJobQueueName(jobKey spec.JobKey) string {
return apiQueueNamePrefix(jobKey.APIName) + jobKey.ID + ".fifo"
}
Expand All @@ -57,7 +57,7 @@ func jobKeyFromQueueURL(queueURL string) spec.JobKey {

jobID := strings.TrimSuffix(dashSplit[len(dashSplit)-1], ".fifo")

apiNameSplit := dashSplit[1 : len(dashSplit)-1]
apiNameSplit := dashSplit[2 : len(dashSplit)-1]
apiName := strings.Join(apiNameSplit, "-")

return spec.JobKey{APIName: apiName, ID: jobID}
Expand Down
3 changes: 1 addition & 2 deletions pkg/types/clusterconfig/aws_policy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ var _cortexPolicy = `
{
"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

},
{
"Effect": "Allow",
Expand Down Expand Up @@ -89,7 +89,6 @@ type CortexPolicyTemplateArgs struct {
LogGroup string
Region string
Bucket string
SQSPrefix string
AccountID string
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/types/clusterconfig/cluster_config_aws.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -682,8 +682,8 @@ func (cc *Config) ToAccessConfig() AccessConfig {
}

func SQSNamePrefix(clusterName string) string {
// 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

}

// returns hash of cluster name and adds trailing "-"
Expand Down