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.

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

Jump to bottom

Open PR to remove providers from module #109

Closed
1 of 4 tasks
gb-ckedzierski opened this issue Aug 28, 2018 · 6 comments
Closed
1 of 4 tasks

Open PR to remove providers from module #109

gb-ckedzierski opened this issue Aug 28, 2018 · 6 comments

Comments

@gb-ckedzierski
Copy link

gb-ckedzierski commented Aug 28, 2018 •
edited

I have issues

I'm submitting a...

  • bug report
  • feature request
  • support request
  • kudos, thank you, warm fuzzy

What is the current behavior?

Currently, because of providers defined at the module level, you cannot delete the module to remove your infrastructure. Conversely, you have to use commands to remove items from the state file before you can delete. See #108

Per these 2 issues, it seems after terraform 0.11 these providers should no longer be in modules.
hashicorp/terraform#16824
hashicorp/terraform#17928

If this is a bug, how to reproduce? Please include a code sample if relevvant.

User Terraform > 0.11 and add the module to create infra, after the apply remove the code, you will get error.

What's the expected behavior?

All infra in this module is deleted.

Are you able to fix this problem and submit a PR? Link here if you have already.

Yes, but don't have permissions to contribute. eg create branch for a PR.

Environment details

  • Affected module version: terraform-aws-eks
  • OS: N/A
  • Terraform version: v0.11.7

Any other relevant info

Something like this should fix the issues, I tested locally and it works on my machine.

provider "null" {
  version = "~> 1.0"
  alias = "default"
}

provider "template" {
  version = "~> 1.0"
  alias = "default"
}

module "eks" {
  source                     = terraform-aws-modules/eks/aws"
  cluster_name          = "development"
  subnets                   = "${module.vpc_app.private_subnets}"
  tags                         = "${map("Environment", "${var.stage}")}"
  vpc_id                      = "${module.vpc_app.vpc_id}"
  manage_aws_auth   = false

  providers = {
    aws = "aws.default"
    null = "null.default"
    template = "template.default"
  }
}

Next remove these lines from main.tf

...
- provider "null" {}
- provider "template" {}
@max-rocket-internet
Copy link
Contributor

Is this really standard practice now? We need to pass providers into every module?

@dpiddockcmp
Copy link
Contributor

Is this really standard practice now? We need to pass providers into every module?

No.

The primary provider instance does not need an alias specified. You only need to use alias if you are dealing with multiple instances of the same provider (e.g. default aws provider is in us-east-1 and you also want to add DR resources in us-west-2).

provider "aws" {
  region = "us-east-1"
}

provider "aws" {
  region = "us-east-2"
  alias  = "dr"
}

The default provider is automatically passed through to modules for their use.

Setting version on providers is not required but is recommended best practice.

It is recommended best practice not to define providers within modules. It is really frustrating from a module provider's PoV as you cannot specify the minimum compatible provider version. This has caused confusion in the community VPC module a few times when e.g. the new tags-on-EIPs feature was used.

Removing null and template should not be a breaking change. Users will get a warning that they should add a provider block with version when doing terraform init

Chris's example is better written as:

provider "null" {
  version = "~> 1.0"
}

provider "template" {
  version = "~> 1.0"
}

module "eks" {
  source          = "terraform-aws-modules/eks/aws"
  cluster_name    = "development"
  subnets         = "${module.vpc_app.private_subnets}"
  tags            = "${map("Environment", "${var.stage}")}"
  vpc_id          = "${module.vpc_app.vpc_id}"
  manage_aws_auth = false
}

@gb-ckedzierski gb-ckedzierski mentioned this issue Oct 15, 2018
Closed
4 tasks
@max-rocket-internet
Copy link
Contributor

Removing null and template should not be a breaking change.

Alright, let's do that now. I'll make a PR.

@max-rocket-internet max-rocket-internet mentioned this issue Oct 16, 2018
Merged
6 tasks
@max-rocket-internet
Copy link
Contributor

How does this look? #168

@max-rocket-internet
Copy link
Contributor

PR is now merged so I'll close this issue.

@github-actions
Copy link

github-actions bot commented Dec 3, 2022

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@max-rocket-internet @dpiddockcmp @gb-ckedzierski