chore: Add custom plan modifier for RequestOnlyRequiredOnCreate Attribute #3977
Conversation
…e requestonlyrequiredonCreate is detected
There was a problem hiding this comment.
Pull request overview
This PR introduces a new custom plan modifier RequestOnlyRequiredOnCreate to improve the user experience when importing resources. The modifier validates that attributes marked with RequestOnlyRequiredOnCreate = true have values during resource creation, while allowing them to be omitted during import operations.
- Renamed
requiredOnCreateInputOnlyTransformationtorequestOnlyRequiredOnCreateTransformationfor consistency - Added
RequestOnlyRequiredOnCreateModifierthat validates required attributes during creation but allows omission on import - Refactored plan modifier generation to support multiple modifiers on a single attribute
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/codegen/codespec/config.go | Renamed transformation function to match the attribute name |
| tools/codegen/gofilegen/schema/schema_attribute.go | Refactored to support multiple plan modifiers and added RequestOnlyRequiredOnCreate modifier generation |
| tools/codegen/gofilegen/schema/schema_attribute_test.go | Added test coverage for RequestOnlyRequiredOnCreate attribute behavior |
| internal/common/customplanmodifier/request_only_required_create.go | New plan modifier implementation that validates required attributes during creation |
| internal/serviceapi/orgserviceaccountapi/resource_schema.go | Applied new plan modifier to secret_expires_after_hours attribute |
| internal/serviceapi/orgserviceaccountsecretapi/resource_schema.go | Applied new plan modifier to secret_expires_after_hours attribute |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| func (m *requestOnlyRequiredOnCreateAttributePlanModifier) MarkdownDescription(ctx context.Context) string { | ||
| return "Ensures that create operations fail when attempting to create a resource with a missing required attribute." |
There was a problem hiding this comment.
The description is misleading. The modifier doesn't make create operations fail - it validates that required attributes are present during creation. Consider revising to: 'Validates that the attribute is provided during resource creation, but allows it to be omitted during import operations.'
| return "Ensures that create operations fail when attempting to create a resource with a missing required attribute." | |
| return "Validates that the attribute is provided during resource creation, but allows it to be omitted during import operations." |
| ) | ||
|
|
||
| // RequestOnlyRequiredOnCreate returns a plan modifier that fails planning if the value is | ||
| // missing (null/unknown) during create (i.e., when state is null), but allows omission on read/import. |
There was a problem hiding this comment.
I believe such a value can only be null/non-null but never unknown right? Since these would be Optional attributes? If yes, I'd remove the Known check in the validation method & only check for null
|
|
||
| // validateRequestOnlyRequiredOnCreate checks that an attribute has a known, non-null | ||
| // value during create and adds an error if it does not. | ||
| func validateRequestOnlyRequiredOnCreate(isCreate bool, planValue attr.Value, attrPath path.Path, diagnostics *diag.Diagnostics) { |
There was a problem hiding this comment.
should we also throw an error if the user tries to update it? Or is the expectation to always use it in conjunction with customplanmodifier.CreateOnly()?
How is it handled during resource reads? I assume the attribute would still stay in the state even though the GET calls don't return it right?
There was a problem hiding this comment.
If the user tries to update it, customplanmodifier.CreateOnly() is responsible for blocking the change and surfacing an error. RequestOnlyRequiredOnCreate only enforces that the attribute is present on create, so for attributes that are both required-on-create and immutable we expect to always use it in conjunction with CreateOnly(). However, an attribute could be CreateOnly, without RequestOnlyRequiredOnCreate.
Regarding your second question: you're correct. On resource reads, the attribute stays in the state, so it retains its last known value.
marcabreracast
deleted the
CLOUDP-363710_RequestOnlyRequiredOnCreate_AddCustomPlanModifier
branch
Description
This PR addresses the following:
requestOnlyRequiredOnCreate, the transformation step function naming needs to match for consistency purposes.RequestOnlyRequiredOnCreateModifier, which adds validation when trying to create a resource with an attribute marked asRequestOnlyRequiredOnCreate = true, an error message will be shown.importwithout having to apply an additional step, improving the user experience.Link to any related issue(s): CLOUDP-363710
Type of change:
Required Checklist:
Further comments
Behaviour of
CreateOnlyattribute is preserved, where the attribute is required on creation, but when tried to be updated in the configuration, a validation error message will be shown.