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

Add wnv to supported pathogen repos #29

Merged
merged 1 commit into from
Oct 1, 2024
Merged

Add wnv to supported pathogen repos #29

merged 1 commit into from
Oct 1, 2024

Conversation

j23414
Copy link
Contributor

@j23414 j23414 commented Oct 1, 2024

Description of proposed changes

Prompted by nextstrain/WNV#13 which includes
the addition of a GH Action workflow that uses pathogen-repo-build workflow.

Checklist

  • Checks pass

Prompted by nextstrain/WNV#13 which includes
the addition of a GH Action workflow that uses pathogen-repo-build
workflow.
@tsibley
Copy link
Member

tsibley commented Oct 1, 2024

In Slack, @j23414 reported she got this error when attempting to apply this change:

aws_iam_policy.NextstrainPathogen["wnv"]: Creating...
aws_iam_role.GitHubActionsRoleNextstrainBatchJobs: Modifying... [id=GitHubActionsRoleNextstrainBatchJobs]
github_actions_repository_oidc_subject_claim_customization_template.nextstrain["wnv"]: Creating...
github_repository_topics.pathogen["wnv"]: Creating...
aws_iam_policy.NextstrainPathogen["wnv"]: Creation complete after 1s [id=arn:aws:iam::827581582529:policy/NextstrainPathogen@wnv]
aws_iam_role.GitHubActionsRoleNextstrainRepo["wnv"]: Creating...
aws_iam_role.GitHubActionsRoleNextstrainRepo["wnv"]: Creation complete after 0s [id=GitHubActionsRoleNextstrainRepo@wnv]
github_actions_repository_oidc_subject_claim_customization_template.nextstrain["wnv"]: Creation complete after 2s [id=wnv]
github_repository_topics.pathogen["wnv"]: Creation complete after 2s [id=wnv]
╷
│ Error: updating IAM Role (GitHubActionsRoleNextstrainBatchJobs) assume role policy: LimitExceeded: Cannot exceed quota for ACLSizePerRole: 2048
│ 	status code: 409, request id: 5b0a1867-8130-4c3e-981c-45d88bf1cd80
│ 
│   with aws_iam_role.GitHubActionsRoleNextstrainBatchJobs,
│   on aws-iam-role-GitHubActionsRoleNextstrainBatchJobs.tf line 7, in resource "aws_iam_role" "GitHubActionsRoleNextstrainBatchJobs":
│    7: resource "aws_iam_role" "GitHubActionsRoleNextstrainBatchJobs" {
│ 
╵

We'll need to adjust how we're defining the role that grew too large.

@tsibley
Copy link
Member

tsibley commented Oct 1, 2024

While we'll eventually need to split that GitHubActionsRoleNextstrainBatchJobs role into two (or otherwise tweak it) to accommodate the role trust policy length limit, in the meantime I've bumped that quota from 2,048 chars to 4,096 chars (the max that's automatically approved).

@j23414 Try again? Also, please merge this branch and push to main before planning and applying the changes.

@j23414 j23414 merged commit 8345e5a into main Oct 1, 2024
1 check passed
@j23414 j23414 deleted the add-wnv branch October 1, 2024 17:48
@j23414
Copy link
Contributor Author

j23414 commented Oct 1, 2024

Works, thank you! :D

terraform -chdir=env/production apply plan

#> aws_iam_role.GitHubActionsRoleNextstrainBatchJobs: Modifying... [id=GitHubActionsRoleNextstrainBatchJobs]
#> aws_iam_role.GitHubActionsRoleNextstrainBatchJobs: Modifications complete after 1s [id=GitHubActionsRoleNextstrainBatchJobs]
#> 
#> Apply complete! Resources: 0 added, 1 changed, 0 destroyed.

@@ -23,6 +23,7 @@ locals {
"rsv" = ["rsv"],
"seasonal-cov" = ["seasonal-cov"],
"seasonal-flu" = ["seasonal-flu"],
"wnv" = ["wnv"],
Copy link
Member

Choose a reason for hiding this comment

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

Coming back here (after an "ah ha" moment in Slack) to note for posterity that this should have used WNV, as the actual repo name (e.g. https://github.com/nextstrain/WNV) and pathogen name (e.g. https://nextstrain.org/WNV and https://data.nextstrain.org/WNV_NA.json) are both capitalized. While GitHub's not case-sensitive when it comes to repo names, everything else involved is.

@j23414 j23414 mentioned this pull request Oct 4, 2024
1 task
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

Successfully merging this pull request may close these issues.

3 participants