Skip to content

IPL-9014 Support name and provider when unconventional display identifier#1959

Merged
jillirami merged 5 commits intomainfrom
jillirami/IPL-9014-tfe-registry-module-support-any-display-identifier
Feb 23, 2026
Merged

IPL-9014 Support name and provider when unconventional display identifier#1959
jillirami merged 5 commits intomainfrom
jillirami/IPL-9014-tfe-registry-module-support-any-display-identifier

Conversation

@jillirami
Copy link
Copy Markdown
Contributor

@jillirami jillirami commented Feb 2, 2026

Description

This pull request enhances the tfe_registry_module resource to support explicit name and module_provider fields when using a VCS repository with a source_directory. This is especially useful for monorepos or repositories that do not follow the standard terraform-<provider>-<name> naming convention. The changes ensure that both name and module_provider are required in these cases, improve validation logic, update documentation, and add comprehensive test coverage for these scenarios.

Enhancements to tfe_registry_module resource:

  • Added support for specifying name and module_provider alongside vcs_repo with source_directory, enabling registration of modules from monorepos or non-standard repository names. [1] [2]
  • Introduced a new validation function (validateNameAndProvider) to enforce that both name and module_provider are provided when the repository name does not follow the terraform-<provider>-<name> convention and source_directory is set. [1] [2]

Schema and validation updates:

  • Removed the restriction that only one of vcs_repo or module_provider can be specified, allowing their combined use when required.
  • Updated error messages and validation logic in acceptance tests to reflect the new requirements for name and module_provider. [1] [2] [3] [4]

Documentation improvements:

  • Added a new example and detailed explanation for creating a registry module from a monorepo with source_directory, highlighting the need to specify both name and module_provider.
  • Clarified documentation on the usage and requirements of module_provider and name fields.

Test coverage:

  • Added new acceptance tests for monorepos with non-standard names and updated existing tests to cover the new validation logic and resource behavior. [1] [2]

Remember to:

Testing plan

resource "tfe_registry_module" "monorepo" {
  organization    = tfe_organization.example.name
  name            = "test"
  module_provider = "aws"

  vcs_repo {
    display_identifier = var.github_registry_module_identifier
    identifier         = var.github_registry_module_identifier
    oauth_token_id     = tfe_oauth_client.github.oauth_token_id
    branch             = var.github_registry_module_branch
    tags               = false
    source_directory   = "modules/null"
  }
}

External links

Output from acceptance tests

Please run applicable acceptance tests locally and include the output here. See testing.md to learn how to run acceptance tests.

jillianne.ramirez@jillianne terraform-provider-tfe % go test -v -run TestAccTFERegistryModule_monorepoNonStandardName ./internal/provider/
=== RUN   TestAccTFERegistryModule_monorepoNonStandardName
--- PASS: TestAccTFERegistryModule_monorepoNonStandardName (7.05s)
PASS
ok  	github.com/hashicorp/terraform-provider-tfe/internal/provider	7.925s
go test -v -run TestAccTFERegistryModule_monorepoNonStandardNameWithoutNameandProvider ./internal/provider/
=== RUN   TestAccTFERegistryModule_monorepoNonStandardNameWithoutNameandProvider
--- PASS: TestAccTFERegistryModule_monorepoNonStandardNameWithoutNameandProvider (0.66s)
PASS
ok  	github.com/hashicorp/terraform-provider-tfe/internal/provider	1.620s
Screenshot 2026-02-19 at 10 48 54 Screenshot 2026-02-20 at 11 57 33

Rollback Plan

Changes to Security Controls

@jillirami jillirami changed the title Support name and provider when display identifier unexpected IPL-9014 Support name and provider when display identifier outside convention Feb 2, 2026
@jillirami jillirami changed the title IPL-9014 Support name and provider when display identifier outside convention IPL-9014 Support name and provider when unconventional display identifier Feb 2, 2026
@jillirami jillirami force-pushed the jillirami/IPL-9014-tfe-registry-module-support-any-display-identifier branch from 9a8fedb to 72e137a Compare February 2, 2026 18:04
Optional: true,
Computed: true,
ForceNew: true,
ExactlyOneOf: []string{"vcs_repo"},
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Required to be removed because this prevents having both vcs_repo AND module_provider which we need to support monorepo scenarios with source_directory

@jillirami jillirami force-pushed the jillirami/IPL-9014-tfe-registry-module-support-any-display-identifier branch from 72e137a to d94fc03 Compare February 2, 2026 18:17
@jillirami jillirami self-assigned this Feb 4, 2026
@jillirami jillirami force-pushed the jillirami/IPL-9014-tfe-registry-module-support-any-display-identifier branch from d94fc03 to e2678b3 Compare February 6, 2026 01:24
@jillirami jillirami force-pushed the jillirami/IPL-9014-tfe-registry-module-support-any-display-identifier branch 2 times, most recently from e2678b3 to c38cbff Compare February 17, 2026 16:40
@datadog-terraform-cloud-hashicorp
Copy link
Copy Markdown

datadog-terraform-cloud-hashicorp bot commented Feb 17, 2026

⚠️ Tests

Fix all issues with Cursor

⚠️ Warnings

❄️ 1 New flaky test detected

TestAccTFEWorkspace_paginatedRemoteStateConsumers from resource_tfe_workspace_test.go (Datadog) (Fix with Cursor)
Step 1/1 error: After applying this test step, the refresh plan was not empty.
stdout


Terraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

...

ℹ️ Info

🧪 All tests passed

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: af38c9e | Docs | Was this helpful? Give us feedback!

@jillirami jillirami force-pushed the jillirami/IPL-9014-tfe-registry-module-support-any-display-identifier branch 19 times, most recently from add5b9e to 40b6992 Compare February 18, 2026 17:09
@jillirami jillirami marked this pull request as ready for review February 18, 2026 17:09
@jillirami jillirami requested a review from a team as a code owner February 18, 2026 17:09
@jillirami jillirami requested a review from zainq11 February 18, 2026 17:24
@jillirami jillirami force-pushed the jillirami/IPL-9014-tfe-registry-module-support-any-display-identifier branch from 40b6992 to b559277 Compare February 19, 2026 14:39
@jillirami jillirami marked this pull request as draft February 19, 2026 14:47
@jillirami jillirami marked this pull request as ready for review February 19, 2026 14:53
@jillirami
Copy link
Copy Markdown
Contributor Author

Failures observed in CI will be resolved with hashicorp/go-tfe#1283 and #1975

Copy link
Copy Markdown
Collaborator

@brandonc brandonc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally speaking, could you add a test for the error case where the convention is not followed and no name or provider is given? That's not blocking feedback, though.

Comment on lines +609 to +610
nameParts := strings.Split(repoName, "-")
followsConvention := len(nameParts) == 3
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This prohibits dashes in the module name, which I think are valid

I think you may need to check for at least 3 parts

Suggested change
nameParts := strings.Split(repoName, "-")
followsConvention := len(nameParts) == 3
nameParts := strings.SplitN(repoName, "-", 3)
followsConvention := len(nameParts) == 3

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for noticing this - atlas will follow-through with any additional validation as well. I will merge your suggestion along with that negative test case.

Copy link
Copy Markdown
Collaborator

@brandonc brandonc Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps the first part should must be "terraform" ? Just thinking out loud and not sure it matters too much

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brandonc great point, I believe here it doesn't matter for the following reasons.

Although you are 100% right we did suggest this pattern of terraform-provider-name, and even having an example that aligns in our guide docs - it is not something that is actually enforced or required in our processing of the registry module (referencing the code that processes it here).

Even in our in-app experience, we are able to create a registry module without the leading terraform
Screenshot 2026-02-20 at 12 24 38

@jillirami jillirami force-pushed the jillirami/IPL-9014-tfe-registry-module-support-any-display-identifier branch from 26b25c0 to 7b81e4b Compare February 20, 2026 16:59
jillirami and others added 2 commits February 20, 2026 12:00
Co-authored-by: Brandon Croft <brandon.croft@gmail.com>
zainq11
zainq11 previously approved these changes Feb 20, 2026
brandonc
brandonc previously approved these changes Feb 23, 2026
@jillirami jillirami dismissed stale reviews from brandonc and zainq11 via af38c9e February 23, 2026 21:42
@jillirami jillirami merged commit 6dd2ffb into main Feb 23, 2026
37 of 39 checks passed
@jillirami jillirami deleted the jillirami/IPL-9014-tfe-registry-module-support-any-display-identifier branch February 23, 2026 22:23
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.

3 participants