MatthewJohn / terrareg

Open source Terraform module registry with UI, optional Git integration and deep analysis
https://gitlab.dockstudios.co.uk/pub/terrareg
GNU General Public License v3.0
268 stars 20 forks source link

Optional module provider_name #38

Closed bmaximuml closed 6 months ago

bmaximuml commented 6 months ago

Hello 👋 ,

All of my modules are being deployed internally on a GKE cluster, and therefore provider_name is always set to null for all the modules I create. It doesn't cause problems, but it feels a bit untidy having it there, especially as it's shown in so many places.

Is it possible to make this optional? Or have a config flag to disable it entirely?

Thanks!

MatthewJohn commented 6 months ago

Hey @bmaximuml,

It seems interesting that this is the case - I would expect, usually, the provider to be set to "kubernetes" or such. For example, if you look at some of the modules in the official registry (e.g. https://registry.terraform.io/modules/iplabs/alb-ingress-controller/kubernetes/latest) they are using "kubernetes" as a provider. Null is generally only used for modules that literally do not interact with anything (e.g. a module that outputs some static information for convenience purposes etc.)

I would generally treat the "null" provider as a provider of any time (it's not a javascript-esque null value), rather the name of some built-in provider (though maybe that's just me). As a result, the question becomes "can we have a default provider, or pre-populate one, if we don't want to set one" - does this seem fair?

Matt

bmaximuml commented 6 months ago

That does make sense from an implementation perspective, mine should be set to kubernetes rather than null. From a usability perspective, I do think it would be nicer without, similar to some in the terraform registry (e.g. https://registry.terraform.io/providers/hashicorp/helm/latest).

Perhaps both could be supported? E.g. for modules with multiple, one could be set as the default, which is what you would get if you pulled {namespace}/{module} rather than {namespace}/{module}/{provider}. For ones, with only one provider, that could naturally be the default, and could then also support both syntaxes

MatthewJohn commented 6 months ago

Hey Max,

I'll have a think about some of the potential issues, but just to clarify:

Matt

bmaximuml commented 6 months ago

You're right about that one, my mistake. Primary concern is the URL that the end user will use in terraform, not too bothered about the other two. A further look through the terraform registry suggests this might just be my incorrect understanding of how modules are exposed in terraform. I do think that it's nicer for a user to access namespace/module if the provider is not really relevant, however if the change I am suggesting would make this inconsistent with all other terraform modules then it's probably not great practice 😂

MatthewJohn commented 6 months ago

If it is (number 3) the URL that people use in modules, then, although I can explore, I suspect we may be a little tied on the matter - Terraform (or other alternatives) are quite regiment in their structure of <registry>/<namespace>/<module>/<provider> and not something we can hack around with too much, unfortunately

Matt

MatthewJohn commented 6 months ago

and, in-fact, if we test, Terraform throws errors with any other structure of URL:

cat main.tf
module "ecs" {
  source  = "local-dev.dock.studio/namespace/modulename"
  version = "5.9.1
}

(venv) ➜  terraform git:(main) ✗ terraform-1.4.5 init

Initializing the backend...
Initializing modules...
â•·
│ Error: Invalid registry module source address
│ 
│ Module "ecs" (declared at main.tf line 30) has invalid source address "local-dev.dock.studio/namespace/modulename": source address must
│ have three more components after the hostname: the namespace, the name, and the target system.
│ 
│ Terraform assumed that you intended a module registry source address because you also set the argument "version", which applies only to registry
│ modules.
╵
bmaximuml commented 6 months ago

That's fair, I appreciate you looking into this. I've clearly just confused providers and modules in my head.

MatthewJohn commented 6 months ago

Created gitlab issue: https://gitlab.dockstudios.co.uk/pub/terrareg/-/issues/511 gitlab-issue-id:511