Skip to content

Importer: emit Terraform that actually imports - #149

Merged
bshutterkion merged 3 commits into
mainfrom
fix/importer-real-world
Aug 25, 2026
Merged

Importer: emit Terraform that actually imports#149
bshutterkion merged 3 commits into
mainfrom
fix/importer-real-world

Conversation

@bshutterkion

@bshutterkion bshutterkion commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

Run against a live Kion (843 resources), the generated configuration and its import script disagreed in seven ways. Every one meant resources that could not be imported — and the last meant a permanent terraform plan diff.

What was broken

1. Nested modules were never declared. OU and project cloud access roles are written into per-parent subdirectories, but Terraform only loads .tf files directly inside a module's source directory. Nothing declared those, so every role in them was invisible: resource address ... does not exist in the configuration. They also need their own provider.tf — a child module that doesn't declare required_providers resolves kion_* against the default namespace and fails with hashicorp/kion.

2. Their import addresses were malformed. import_ou_roles built module.ou-cloud-access-role.kion.<ou_dir>_ou_cloud_access_role.<name>, splicing the directory into the resource type. import_project_roles omitted the module segment entirely, collapsing three different Developer roles in three different projects onto one address.

3. Colliding names silently destroyed data. Two CloudFormation templates both named CIS-EnableVPCFlowLogs (ids 81, 182) normalised to one label, so the second .tf overwrote the first — one resource vanished from the configuration — while both claimed the same import address. Same for two IAM policies named deny-non-tagged-resources. unique_label() appends the Kion id on collision; the first to claim a name keeps the clean label.

4. A leading digit produced invalid HCL. A block label may start with a digit because it is quoted; the reference emitted beside it may not, so a resource named 800-53 Audit made Terraform reject the whole file. This also affected every name under --prepend-id, which always prepends a digit.

5. aws_permissions_boundary_id — a name neither provider has ever accepted. Both call it aws_iam_permissions_boundary.

6. Ownership was looked up by a hardcoded app_role_id == 1. App roles are installation-specific. On the instance tested, ownership was a custom role named Owner with id 67 while id 1 was the system-managed Admin; 14 project mappings used role 1 and 26 used role 67. Now resolved by role name, taking the union of Owner and Admin. OUs get a second source: /v3/ou/{id} returns owner_users: null for OUs whose ownership is a permission mapping, so it falls back to the mapping.

7. process_string collapsed whitespace and rewrote quotes. Descriptions are markdown with meaningful indentation; collapsing \s{2,} to a single space meant the configuration disagreed with the API on every indented line, permanently. Double quotes were replaced with single quotes for the same reason. Now escaped properly — indentation, embedded quotes, backslashes and ${...} all round-trip.

Verified end to end

before after
import statements / distinct addresses 843 / 829 843 / 843
failures in a 56-resource sample 17 0
parse errors across 850 files 2 0
projects with no inferable owner 11 of 37 0

Validated end to end on that instance: the importer enumerates the installation, and every one of the 843 emitted terraform import statements resolves to an address that exists in the generated configuration and imports cleanly.

Run against a live instance (843 resources), the generated configuration and
its import script disagreed in four ways. Every one of them meant resources
that could not be imported at all.

1. Nested modules were never declared. OU and project cloud access roles are
   written into per-parent subdirectories, but Terraform loads only the .tf
   files directly inside a module's source directory — never subdirectories.
   Nothing declared those, so every role in them was invisible to Terraform
   and its import failed with "resource address ... does not exist in the
   configuration". Each subdirectory is now declared as a module of its own.

2. The import addresses for those roles were malformed. import_ou_roles built
   "module.ou-cloud-access-role.kion.<ou_dir>_ou_cloud_access_role.<name>",
   splicing the directory into the resource TYPE instead of emitting a module
   segment; import_project_roles omitted the segment entirely. The latter
   collapsed three different "Developer" roles in three different projects
   onto one address, so two of them could never be imported.

3. Kion names are not unique, and colliding names silently destroyed data. Two
   CloudFormation templates both named "CIS-EnableVPCFlowLogs" (ids 81, 182)
   and two IAM policies both named "deny-non-tagged-resources" (790, 791)
   normalized to one label, so the second .tf file overwrote the first — one
   resource vanished from the configuration — while both claimed the same
   import address. unique_label() appends the Kion id on collision; the first
   resource to claim a name keeps the clean label, so configurations without
   duplicates are unchanged.

   This also fixes a divergence in the same code: the filename honoured
   --prepend-id while the import address did not, so every address that flag
   produced was wrong. Both now come from one label.

4. A leading digit produced invalid HCL. A block label may start with a digit
   because it is quoted; the reference emitted beside it may not, so a Kion
   resource named "800-53 Audit" made Terraform reject the whole file. Also
   affected every name under --prepend-id, which always prepends a digit.

Verified against the live instance: 843 import statements, 843 distinct
addresses, 843 resource blocks — an exact 1:1, where before it was 843
statements against 829 addresses with 17 failures in a 56-resource sample.
process_string escaped newlines and then collapsed every run of whitespace to
a single space. Kion descriptions are markdown and carry meaningful
indentation, so the imported configuration disagreed with the API on every
indented line:

  API:    "    The ARN of the Central Auditing Event Bus."
  config: " The ARN of the Central Auditing Event Bus."

Terraform therefore reported a description difference on every plan, forever,
for an attribute nobody had changed. It was the last remaining diff after a
full 843-resource migration test — everything else reached zero.

It also replaced double quotes with single quotes to avoid terminating the
HCL string, which silently rewrote the value for the same reason. Escaping
preserves it.

Escape properly instead: backslash first so later escapes are not themselves
re-escaped, then CR/LF/tab, then the double quote, then ${ as $${ since HCL
interpolates inside a quoted string. Verified that indentation, embedded
double quotes, backslashes and ${...} all round-trip — what Terraform reads
back now equals what the API returned.
kion_project and kion_ou both require at least one of owner_user_ids /
owner_user_group_ids, so a resource imported without owners produces
configuration Terraform refuses to load.

Owners were read from the project's permission mapping by looking for
app_role_id == 1, on the assumption that role 1 is always Admin and Admin
always means owner. Neither half holds: app roles are installation-specific.
On the instance this was tested against, ownership was a CUSTOM role named
"Owner" with id 67 while id 1 was the system-managed "Admin" — 14 project
mappings used role 1 and 26 used role 67, so the hardcoded check silently
found no owners for most projects.

Resolve the roles by NAME instead, taking the union of "Owner" and "Admin"
because an installation can map some resources through each. Accumulate
across every matching role rather than stopping at the first.

OUs need a second source: /v3/ou/{id} returns owner_users: null for OUs whose
ownership is expressed as a permission mapping, so fall back to the mapping
when the detail endpoint yields nothing.

Also declares each nested per-parent directory as a module WITH its own
provider.tf. A child module that does not declare required_providers resolves
kion_* against the default namespace and fails with "hashicorp/kion", so the
module declaration alone was not enough.

Projects with no inferable owner went from 11 of 37 to 0.
@bshutterkion
bshutterkion requested a review from egramens August 25, 2026 04:39
@bshutterkion
bshutterkion merged commit 9d086ec into main Aug 25, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants