VAULT-43393 Initial implementation for config control group resource by johnjosephg · Pull Request #2840 · hashicorp/terraform-provider-vault

johnjosephg · 2026-03-23T18:38:40Z

Description

Initial implementation for the vault_config_control_group resource in /sys/config/control-group.

Checklist

Added CHANGELOG entry (only for user-facing changes)
Acceptance tests where run against all supported Vault Versions

Output from acceptance testing:

go test -v ./internal/vault/sys/config -run 'TestAccControlGroupConfig' -count=1
=== RUN   TestAccControlGroupConfigResourceSchema
=== PAUSE TestAccControlGroupConfigResourceSchema
=== RUN   TestAccControlGroupConfigResource
--- PASS: TestAccControlGroupConfigResource (0.93s)
=== RUN   TestAccControlGroupConfigResourceNamespace
--- PASS: TestAccControlGroupConfigResourceNamespace (1.30s)
=== CONT  TestAccControlGroupConfigResourceSchema
--- PASS: TestAccControlGroupConfigResourceSchema (0.00s)
PASS

Community Note

Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

PCI review checklist

I have documented a clear reason for, and description of, the change I am making.
If applicable, I've documented a plan to revert these changes if they require more than reverting the pull request.
If applicable, I've documented the impact of any changes to security controls.

Examples of changes to security controls include using new access control methods, adding or removing logging pipelines, etc.

Balaji2198

Thanks for working on this. This resource implementation is currently developed in the SDKv2, but new resources should be built using the Terraform Plugin Framework. Please update the implementation accordingly. You can get the reference from the internal/vault/sys/password_policy.go

Balaji2198

Thanks for working on this, I’ve added a few comments.

vijayavelsekar · 2026-04-09T09:32:25Z

+
+func (r *ControlGroupConfigResource) Schema(_ context.Context, _ resource.SchemaRequest, resp *resource.SchemaResponse) {
+	resp.Schema = schema.Schema{
+		MarkdownDescription: "Manages the singleton control group configuration. Requires Vault Enterprise 1.10.0 or later.",


I don't think we need to mention the version requirement, because TFVP itself is supported from vault version 1.15.x.

vijayavelsekar · 2026-04-09T09:32:38Z

+		Attributes: map[string]schema.Attribute{
+			consts.FieldMaxTTL: schema.StringAttribute{
+				Optional:            true,
+				MarkdownDescription: "The maximum ttl for a control group wrapping token. This can be provided in seconds or duration (for example, 2h). Requires Vault Enterprise 1.10.0 or later.",


I don't think we need to mention the version requirement, because TFVP itself is supported from vault version 1.15.x.

vijayavelsekar · 2026-04-09T09:33:39Z

+// featureSupported checks if the Vault server supports control group configuration.
+// This feature requires Vault Enterprise 1.10.0 or later.
+// The function takes an error callback to allow flexible error reporting across different contexts.
+func (r *ControlGroupConfigResource) featureSupported(addError func(summary string, detail string)) bool {


I don't think we need version gating, because TFVP itself is supported from vault version 1.15.x.

vijayavelsekar · 2026-04-09T09:35:13Z

+	}
+
+	// Check if Vault is Enterprise edition (control groups are an Enterprise feature)
+	if !provider.IsEnterpriseSupported(r.Meta()) {


We don't have to check if its enterprise supported as well. These kind of validations are already in vault and we can let that take care of it.

vijayavelsekar · 2026-04-09T10:02:08Z

+		MarkdownDescription: "Manages the singleton control group configuration. Requires Vault Enterprise 1.10.0 or later.",
+		Attributes: map[string]schema.Attribute{
+			consts.FieldMaxTTL: schema.StringAttribute{
+				Optional:            true,


Should we make this as Required field, because without this we are getting error as max_ttl must be greater than 0 right ?

vijayavelsekar · 2026-04-09T10:05:28Z

+	}
+
+	// Normalize the max_ttl to seconds (e.g., "2h" becomes "7200")
+	maxTTL, err := normalizeMaxTTL(data.MaxTTL.ValueString())


There is no need of normalization, as the api accepts string field itself.

vijayavelsekar · 2026-04-09T10:07:43Z

+		},
+	}
+
+	base.MustAddLegacyBaseSchema(&resp.Schema)


We can use MustAddBaseSchema instead of this because we are not migrating from the old SDK V2.
For ID field, you can have a computed ID field added to the schema.

vijayavelsekar · 2026-04-09T10:11:44Z

+		return
+	}
+
+	if err := r.readIntoModel(ctx, vaultClient, &data); err != nil {


Instead of using this, can we reuse the Read function which already have these logic ?

vijayavelsekar · 2026-04-09T10:12:12Z

+		return
+	}
+
+	if err := r.readIntoModel(ctx, vaultClient, &data); err != nil {


Instead of using this, can we reuse the Read function which already have these logic ?

vijayavelsekar · 2026-04-10T09:04:18Z

+
+// createOrUpdate is a shared helper function for Create and Update operations.
+// The isCreate parameter determines which error message to use for write failures.
+func (r *ControlGroupConfigResource) createOrUpdate(ctx context.Context, req interface{}, resp interface{}, isCreate bool) {


I see that you are trying to call the same function for create and update for reusability, but still you see it has separate control flow for create and update.
My thought was to use the same code for both Create and Update
Also I can see that Read can also be reused.
I've attached the sample code that I generated, which reused Create, update and read code. This will show you what I really mean. But I have not tested this code, probably it might have some issues. Just attaching it for your reference.

control_group_resource.txt

vijayavelsekar · 2026-04-15T03:28:58Z

+  The `namespace` is always relative to the provider's configured [namespace](/docs/providers/vault#namespace).
+   *Available only for Vault Enterprise*.
+
+* `max_ttl` - (Optional) The maximum TTL for a control group wrapping token. This value can be specified as a duration string (e.g., "24h", "1h30m") or as an integer number of seconds (e.g., "86400").


Max ttl is required as per the schema defenition.

vijayavelsekar

LGTM

Removed duplicate sysconfig.NewControlGroupConfigResource entry from the resource list.

johnjosephg marked this pull request as ready for review March 24, 2026 05:52

johnjosephg requested review from a team as code owners March 24, 2026 05:52

johnjosephg requested a review from arjun-sharma-5 March 24, 2026 05:52

Balaji2198 reviewed Mar 25, 2026

View reviewed changes

Balaji2198 reviewed Apr 7, 2026

View reviewed changes

Comment thread internal/vault/sys/config/control_group_resource.go Outdated

Comment thread internal/vault/sys/config/control_group_resource_test.go

Comment thread internal/vault/sys/config/control_group_resource.go

vijayavelsekar reviewed Apr 9, 2026

View reviewed changes

Comment thread internal/vault/sys/config/control_group_resource.go

vijayavelsekar reviewed Apr 10, 2026

View reviewed changes

vijayavelsekar reviewed Apr 15, 2026

View reviewed changes

vijayavelsekar previously approved these changes Apr 15, 2026

View reviewed changes

johnjosephg dismissed vijayavelsekar’s stale review via 8f5d38d April 15, 2026 08:42

Rebased changes

32d3827

johnjosephg force-pushed the VAULT-43393-config-control-group branch from ee1bfbf to 32d3827 Compare April 21, 2026 06:39

johnjosephg added 2 commits April 21, 2026 12:26

Remove duplicate ControlGroupConfigResource from provider

631e77a

Removed duplicate sysconfig.NewControlGroupConfigResource entry from the resource list.

Merge branch 'main' into VAULT-43393-config-control-group

acc9537

Conversation

johnjosephg commented Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Output from acceptance testing:

Community Note

PCI review checklist

Uh oh!

Balaji2198 left a comment

Choose a reason for hiding this comment

Uh oh!

Balaji2198 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vijayavelsekar Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vijayavelsekar left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

johnjosephg commented Mar 23, 2026 •

edited

Loading

vijayavelsekar Apr 9, 2026 •

edited

Loading