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

-var atlantis_repo cannot contain '/' (0.4.9) #295

Closed
jolexa opened this issue Sep 28, 2018 · 4 comments
Closed

-var atlantis_repo cannot contain '/' (0.4.9) #295

jolexa opened this issue Sep 28, 2018 · 4 comments
Labels
feature New functionality/enhancement

Comments

@jolexa
Copy link
Contributor

jolexa commented Sep 28, 2018

-var atlantis_repo=runatlantis/atlantis is invalid per the documentation. It cannot contain a /

Maybe it should be replaced with another character? Or just drop the organization and just use the repo name?

@jolexa jolexa changed the title -var atlantis_repo cannot contain '/' -var atlantis_repo cannot contain '/' (0.4.9) Sep 28, 2018
@lkysow
Copy link
Member

lkysow commented Sep 28, 2018

I don't think it makes sense to change the value of atlantis_repo to match the allowed chars for session_name because I can see others wanting to make use of this variable for different reasons.

What about instead using a tf function to remove the /?

provider "aws" {
  assume_role {
    role_arn     = "arn:aws:iam::ACCOUNT_ID:role/ROLE_NAME"
    session_name = "${var.atlantis_user}-${replace(var.atlantis_repo, "/[^\\w+=,.@-]/", "-")}-${var.atlantis_pull_num}"
  }
}

@jolexa
Copy link
Contributor Author

jolexa commented Sep 28, 2018

My initial reaction is that I do not think it is great that Atlantis fails to assume a role by default ;) I cannot get onboard with having a regex in every repo at our org, not because we don't understand it but rather it is just too ugly (subjective, I know). I would rather implement either of:

  • In our org, we just wouldn't use atlantis_repo and hardcode the repo name as another variable (This defeats the point of the feature but fine for us in our usage of atlantis)
  • Implement another variable in the code that does the split for us. (Possibly using SplitRepoFullName )
    func SplitRepoFullName(repoFullName string) (owner string, repo string) {

@lkysow
Copy link
Member

lkysow commented Sep 28, 2018

That's fair. I like your second suggestion. Also set:
atlantis_repo_name=atlantis
atlantis_repo_owner=runatlantis
And use those.

@lkysow
Copy link
Member

lkysow commented Oct 2, 2018

Fixed by #300

@lkysow lkysow closed this as completed Oct 2, 2018
@lkysow lkysow added the feature New functionality/enhancement label Apr 9, 2019
jamengual pushed a commit that referenced this issue Nov 24, 2022
…nate fetching checkruns for every status update (#290)" (#295)

This reverts commit 426597b.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New functionality/enhancement
Projects
None yet
Development

No branches or pull requests

2 participants