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

Allow import blocks within child modules #33474

Open
kmoe opened this issue Jul 4, 2023 · 31 comments
Open

Allow import blocks within child modules #33474

kmoe opened this issue Jul 4, 2023 · 31 comments

Comments

@kmoe
Copy link
Member

kmoe commented Jul 4, 2023

Currently, import blocks may only appear in the root module. Should we allow them to appear in child modules?

Related: #33376

@Shocktrooper

This comment was marked as off-topic.

@dylan-shipwell

This comment was marked as off-topic.

@arnvid

This comment was marked as off-topic.

jkburges referenced this issue Oct 17, 2023
* website: plannable import docs

* website: config gen docs

* Update website/docs/cli/commands/plan.mdx

Co-authored-by: Rose M Koron <[email protected]>

* Update website/docs/cli/import/index.mdx

Co-authored-by: Rose M Koron <[email protected]>

* Update website/docs/language/import/index.mdx

Co-authored-by: Rose M Koron <[email protected]>

* Update website/docs/language/import/index.mdx

Co-authored-by: Rose M Koron <[email protected]>

* Apply suggestions from code review

Co-authored-by: Rose M Koron <[email protected]>

* fix docs rendering

* Apply suggestions from code review

Co-authored-by: Rose M Koron <[email protected]>

* link again to import blocks

* fix genconfig example plan output

* Update website/docs/language/import/index.mdx

Co-authored-by: Rose M Koron <[email protected]>

* add import resource config example

* Apply suggestions from code review

Co-authored-by: Rose M Koron <[email protected]>

* attempt to fix nav

* more explicit

* fix build?

* remove pseudo tutorial

* add advice on when to gen

* add note on idempotency

* Apply suggestions from code review

Co-authored-by: Rose M Koron <[email protected]>
Co-authored-by: Alan Szlosek Jr <[email protected]>

* refer to cli cmd in usual way

* more explanation for genconfig

* remove unnecessary sentence

* add heading

* update help text

* Apply suggestions from code review

Co-authored-by: Rose M Koron <[email protected]>

* update link

* add import ID section

* Apply suggestions from code review

Co-authored-by: rita <[email protected]>
Co-authored-by: Rose M Koron <[email protected]>

* Apply suggestions from code review

Co-authored-by: rita <[email protected]>
Co-authored-by: Rose M Koron <[email protected]>

* dial back didacticism

* clarify genconfig instructions

* explicit explanation of arg conflict

* Apply suggestions from code review

Co-authored-by: rita <[email protected]>
Co-authored-by: Rose M Koron <[email protected]>

* clarify import block required for genconfig

---------

Co-authored-by: Rose M Koron <[email protected]>
Co-authored-by: Alan Szlosek Jr <[email protected]>
Co-authored-by: rita <[email protected]>
@AMEvers

This comment was marked as duplicate.

@omarismail

This comment was marked as off-topic.

@arnvid

This comment was marked as off-topic.

@markwellis

This comment was marked as duplicate.

@GeorgeGkinis

This comment was marked as off-topic.

@TrueCyberAxe
Copy link

I would also like to be able to use it within child modules, I hadn't realised it was only for the root module and have spent the last few weeks implementing imports into a number of modules so we can update them and move away from legacy code since some of our modules date back to terraform 0.12 before we had an internal standard set and such.

We also have other scenarios that would be solved with the ability to use them specifically for the following two scenarios

Changing Security Groups/Rules without it deleting and re-creating, this has been an annoying blocker for a lot of people within the Company.

Datadog Monitors for example we not too long ago had the Data Dog monitors all moved into a single company wide environment isntead of a different one for each aws environment

A lot of builds stopped working and had to have monitors manually deleted so they could be recreated by the module.

@dylan-shipwell
Copy link

off topic, work around
in case it helps anyone

i don't really believe hcl import will be a successor to proper state migrations. (even if they reverse their opinion on child import statements being illegal)

there aren't any great terraform migration frameworks out there that i know of. there is https://github.com/minamijoyo/tfmigrate but it struggles to accomplish obvious things in a lot of ways similar to terraform native import does.

for now, i'm sticking to manual migration script files
these generally read like this

#!/bin/sh -eux
# terraform/myproject/migrations/0001-some-state-migration.sh
: "${TERRAFORM_BIN:=terraform}" # use "terraform" from $PATH or $TERRAFORM_BIN if specified

do_forward=1
while [[ $# -gt 0 ]]; do
  case $1 in
    --reverse) do_forward=0;;
    *) echo "Unknown option $1"; exit 1;;
  esac
  shift
done

fn_tf_forward() {
    $TERRAFORM_BIN import foo.bar.baz my_uid
}

fn_tf_backward() {
    $TERRAFORM_BIN state rm foo.bar.baz my_uid
}

if [ $do_forward -eq 1 ]; then
    fn_tf_forward; 
else
    fn_tf_backward;
fi

the most annoying thing about this low tech solution is that there is no persistent database of which migration files have been completed. i just remember with my mind right now.

hcl import is nice because it shows a plan of what should happen if both the import step and apply step are happy, this is a great feature, and it remembers if an import is complete.

tfmigrate is nice because it remembers which migrations have been applied.

the nice thing about manual migration scripts is that you can write them in any language you like, meaning i have access to for loops, and if this then that logic control, and string format/templates, not to mention service provider apis like cloud vendors. very helpful when i have 1000 users of a module that aren't all homogeneous but are patterned. given that its also software, i could use these to generate correct import hcl blocks, and i have considered doing this, the most annoying part of this to me, is that the import hcl files must be in the same folder as the top level terraform.tf/main.tf, so i usually move them to a mgirations/ subfolder after they have been applied to not clutter the project. scripts that I run can live anywhere so i prefer that slightly.

@carloprone
Copy link

I also have a scenario where this feature would be very useful.
We have some child modules that dynamically crates (or not) and manages (or not) several resources depending on the environment they are deployed. This is controlled in the root module by passing some flags.
Some of these resources also need to be imported, and at the moment the only way to make this work is to have several complicated import blocks in the root module, where we use the for_each construct to replicate the flag-controlled configuration accordingly.
If we could push the import in the child module, our configuration would be way simpler, easier to maintain and less prone to errors.

Of course, the ID of the imported resource would be an input variable for the child module, while at the moment import blocks only accept literal strings. I can imagine this being a challenge, but it should be doable to relax such constraint.

@nikolaifa
Copy link

Another use case I'd love to be covered.
We have a module managing Azure network resources, due to how the azurerm provider implements some of its resources, and how we want to use Azure governance tooling, we cannot use the azurerm provider for a subset of these resources.
To bypass this limitation I want to use the Microsoft provided azapi provider. Since our module is widely used internally I'm interested in doing the state operations within the module itself to avoid the operational overhead of having to do it manually for each root module.

If I could use both the removed and imported block within the module itself, I can use the resource ID from the already deployed Terraform object to import the new one.

Can you give any sort of time frame for when such a feature will be available, or if it will be?

@oliverwiegers

This comment was marked as off-topic.

@h2oearth

This comment was marked as off-topic.

@omarismail

This comment was marked as off-topic.

@omarismail

This comment was marked as off-topic.

@Blefish

This comment was marked as off-topic.

@lijok
Copy link

lijok commented May 29, 2024

Second. Here's where I'm struggling;

locals {
  service_linked_roles_file               = "${path.module}/service-linked-roles.json"
  service_linked_roles                    = jsondecode(file(local.service_linked_roles_file))
  service_linked_roles_service_principals = keys(local.service_linked_roles)

  service_linked_roles_to_provision = var.service_linked_roles_to_provision == ["*"] ? local.service_linked_roles_service_principals : var.service_linked_roles_to_provision
}

data "aws_caller_identity" "this" {}

resource "aws_iam_service_linked_role" "this" {
  for_each = toset(local.service_linked_roles_to_provision)

  aws_service_name = each.key
  tags             = var.tags

  lifecycle {
    ignore_changes = [
      # Description is auto-generated by AWS.
      description
    ]
  }
}

# Currently importing inside child modules is not supported: https://github.com/hashicorp/terraform/issues/33474
# import {
#   # Some of the service linked roles get automatically created by AWS when an account is created.
#   for_each = toset(local.service_linked_roles_to_provision)
#
#   to = aws_iam_service_linked_role.this[each.key]
#   id = "arn:aws:iam::${data.aws_caller_identity.this.account_id}:role/aws-service-role/${each.key}/${local.service_linked_roles[each.key]}"
# }

@robson90

This comment was marked as duplicate.

@YevhenLodovyi

This comment was marked as off-topic.

@findajay

This comment was marked as off-topic.

@cah-louis-verzi

This comment was marked as off-topic.

@markwellis

This comment was marked as off-topic.

@zackarydev

This comment was marked as off-topic.

@kuzmik

This comment was marked as duplicate.

@bukzor
Copy link

bukzor commented Nov 19, 2024

I'll add my use case here. Maybe it will clarify the implementation and/or priority.

I'm working in GCP, creating a generic_project module that I want my team to use for all our GCP projects. There's a feature in GCP to enable querying logs with SQL with bigquery, but it requires modification of an object that's implicitly created by the project resource.

module "project_factory" {
  source  = "terraform-google-modules/project-factory/google"
  version = "~> 14.5"

  ...
}

locals {
  project_id         = one(module.project_factory[*].project_id)
}

resource "google_logging_project_bucket_config" "default" {
  project   = "projects/${local.project_id}"
  location  = "global"
  bucket_id = "_Default"

  enable_analytics = true   # a non-default value
  retention_days   = 30
  description      = "Default bucket"
}

resource "google_logging_linked_dataset" "default" {
  link_id = "logging_default"

  bucket   = google_logging_project_bucket_config.default.bucket_id
  parent   = google_logging_project_bucket_config.default.project
  location = google_logging_project_bucket_config.default.location
}

This "works" but I'm unsatisfied that the google_logging_project_bucket_config.default object is modified without any review of the plan, plus the fact that it's created out-of-band with respect to terraform-apply isn't clearly represented here. I had hoped to fix both problems by adding:

import {
  to = google_logging_project_bucket_config.default
  id = "projects/${local.project_id}/locations/global/buckets/_Default"
}

but I get this error during plan

│ Error: Invalid import configuration
│
│   on ../../module/generic_project/logging.tf line 33:
│   33: import {
│
│ An import block was detected in "module.project--my_project". Import
│ blocks are only allowed in the root module.

@lantoli
Copy link

lantoli commented Nov 19, 2024

we would also need to allow import inside modules, more info here: hashicorp/terraform-plugin-framework#1058.

Our main use case is to help our customers to move a resource from an old type to a new one using a combination of removed and import blocks.

Right now removed and moved blocks are allowed inside modules, I think there is not any technical issue to allow import as well.

In fact we can use import from the root modules for resources inside the module. The problem with this is that all module clients need to do the import breaking encapsulation, when it should be possible to be done inside the module.

@jbardin
Copy link
Member

jbardin commented Nov 21, 2024

@lantoli, It's unlikely Terraform is going to implement the same mechanism in another way. The absolute minimal implementation of MoveResourceState is in effect functionally identical to manually removing and importing the resource.

Putting an import within a module always breaks encapsulation in some way, because the behavior of the module now depends on external state typically known only by the caller. At a minimum that is going to require a mechanism for dynamic conditional imports, and is the primary reason why this has not yet been implemented.

@dylan-shipwell
Copy link

dylan-shipwell commented Nov 21, 2024

re

require a mechanism conditional imports

suggestion,

the condition can pretty simply be "if the child module source code declare an import block, imports are active".

if we were trying to reuse a module that declares import, an import statement will probably next-to-never make sense, to guard against being stuck needing to use terraform modules that are authored incorrectly that we can't modify; it would make sense to me to disable this feature at the module call site, something like

module "foo" {
  # ...
  lifecycle = {
    allow_imports = False # suggestion: disable/enable import statements if you do/don't want them
  }
}

a sane-default might be to default-allow imports on local modules source = "./..." and disable it on remote modules that are fetched at init time.

@bukzor

This comment was marked as duplicate.

@bukzor

This comment was marked as duplicate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests