Examples using atlantis_repo_allowlist output don't work

[bodgit]

Description

Since #221 was merged, the examples of using the atlantis_repo_allowlist output as an input to the GitHub webhook submodule don't work. Consider the following stripped down example:

module "atlantis" {
  ...
  atlantis_repo_allowlist = ["github.com/someuser/*"]
  ...
}

module "github_repository_webhook" {
  ...
  atlantis_repo_allowlist = module.atlantis.atlantis_repo_allowlist
  ...
}

This code is basically what is in the complete GitHub example in this repo. However, it generates this error:

╷
│ Error: POST https://api.github.com/repos/someuser/github.com/someuser/*/hooks: 404 Not Found []
│ 
│   with module.github_repository_webhook.github_repository_webhook.this[0],
│   on .terraform/modules/github_repository_webhook/modules/github-repository-webhook/main.tf line 7, in resource "github_repository_webhook" "this":
│    7: resource "github_repository_webhook" "this" {
│ 
╵

Initially I thought it was just the wildcard causing the problem, but it's passing in the whole "github.com/.../..." string. You need to pass the short repository name, which will be owned by the account that owns the credentials configured in the GitHub provider.

This used to work as it was using the atlantis_allowed_repo_names variable which would've been configured like ["repo1", "repo2"], etc.

IMHO, removing an input parameter from the module should've been a major version bump here as it breaks backwards compatibility.

Versions

• Terraform: 1.1.3

• Provider(s): aws 3.74.2

• Module: 3.9.0+

Reproduction

See above.

Code Snippet to Reproduce

See above.

Expected behavior

Webhook should be created.

Actual behavior

GitHub API error.

[bryantbiggs]

see #221 - @snovikov care to comment?

[snovikov]

@bodgit thanks for mentioning it. Yes, it seems that this example is misleading. I agree, major version bump would be better.
Unfortunately this test-case was not caught by github-action piplines :(

The reason why atlantis_allowed_repo_names was removed - it was actually not used in the module itself and the only place where you can convienently use it - this example as an output of main module.

On the other hand - atlantis_repo_allowlist (main modules) and atlantis_repo_allowlist (sub-module) ought not to be the same. Consider this example:

module "atlantis" {
  ...
  # Allow all repositories in my account
  atlantis_repo_allowlist = ["github.com/someuser/*"]
  ...
}

module "github_repository_webhook" {
  ...
  # Create webhooks only for specific repositories
  atlantis_repo_allowlist = ["repo1", "repo2"]
  ...
}

But in some cases they could intersect:

locals {
  owner        = "someuser"
  repositories = ["repo1", "repo2"]
  allow_filter = [ for r in loca.repositories : "github.com/${owner}/${r}" ]
}

module "atlantis" {
  ...
  # Allow list repositories in my account
  atlantis_repo_allowlist = local.allow_filter
  ...
}

module "github_repository_webhook" {
  ...
  # Create webhooks only for specific repositories
  atlantis_repo_allowlist = local.repositories
  ...
}

Generally speaking in some cases it is easy to use wildcard, but in some cases - a fixed list.
Should we not better fix the example?

I also think we should rethink names for those parameters in the future, as they are misleading.

[bryantbiggs]

I believe this is what I pointed out here, no? https://github.com/terraform-aws-modules/terraform-aws-atlantis/pull/221/files#r779863857

the webhook sub-module needs to know each repository so it can create the webhook, it can't take a glob pattern

[snovikov]

@bryantbiggs correct. But you need it only for submodules.
My point here is, that neither of submodules (github-repository-webhook or gitlab-repository-webhook) are actually used in the main module - therefore, why do you need this parameter if it is not used at all? The only case which I see here - above mentioned example, which could be easily rewritten.

We have to distiguish those to paramters:

• filter for permissions (glob pattern or repository) - used in the main module and part of atlantis configuration (https://www.runatlantis.io/docs/server-configuration.html#repo-allowlist)
• list of the repositories where webhooks are created - created on demand and separately from atlantis main module.

Maybe rename those input paramters according to their usage to avoid confusion? e.g.

• github_repository_webhook.atlantis_repo_allowlist -> github_repository_webhook.repositories
• gitlab-repository-webhook.atlantis_repo_allowlist -> gitlab-repository-webhook.repositories

[bryantbiggs]

I don't follow. Your PR made the change in the example which is doing the opposite of what you are saying https://github.com/terraform-aws-modules/terraform-aws-atlantis/pull/221/files#diff-f876b6951de7de1c9c2d51b48b33502787d5b450d3dff6f740f5e2ce47043e19R104

The webhook sub-modules are not used in the root module - correct; but they need to take in a list of repositories. So why did you take the glob pattern syntax from the root and push it down to the webhook sub-modules which brakes the functionality? Your PR should have only touched the root module and not the sub-modules

[jeff51419]

atlantis_repo_allowlist = ["repo1","repo2","repo3"]

locals {
    repositories = flatten([
      for repo in var.atlantis_repo_allowlist:
        ["github.com/${var.github_owner}/${repo}"]
    ])
  }

output "atlantis_repo_allowlist" {
  description = "Git repositories where webhook should be created"
  // value       = var.atlantis_repo_allowlist
  value          = local.repositories
}

module "atlantis" {
  
  ...
  atlantis_repo_allowlist    = local.repositories
  ...


module "github_repository_webhook" {
  ...
  atlantis_repo_allowlist = module.atlantis.atlantis_repo_allowlist
  ...
}

we still use module.atlantis.atlantis_repo_allowlist on github_repository_webhook module.
I have no idea why here use "atlantis_repo_allowlist = ["github.com/${var.github_owner}/*"]" in module.
we usually only have one or two terraform or terragrunt repos in Github, and we should not monitor all repos in our organizations.

[bodgit]

Using "*" doesn't cause Atlantis to monitor all repositories. AIUI it's just used as a whitelist for which repositories are allowed to trigger the payload URL. The webhook is what determines if a repository is using Atlantis or not, which needs to be created per repository; it's not a valid API call to pass "*".

The change you've proposed doesn't fix anything as you still end up passing to the webhook module repository names of the form "github.com/.../...", which doesn't work.

[snovikov]

@bodgit, @bryantbiggs I think we can close this issue, as @skalinets was kind and have fixed it in #277.

[github-actions]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.