-
Notifications
You must be signed in to change notification settings - Fork 128
Add config.Resource.OverrideScalarFieldType #724
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
base: main
Are you sure you want to change the base?
Changes from all commits
4bb37a5
e5bad30
ff5bdd1
7bffb1d
2084422
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,7 @@ package config | |
| import ( | ||
| "context" | ||
| "fmt" | ||
| "go/types" | ||
| "strings" | ||
| "time" | ||
|
|
||
|
|
@@ -707,6 +708,11 @@ type Resource struct { | |
| // the Terraform Plugin SDKv2 client. | ||
| useTerraformPluginFrameworkClient bool | ||
|
|
||
| // overrideGeneratedFieldType allows to manually override the type for the | ||
| // generated field of a Resource at the specified Terraform path. | ||
| // We only support type overrides for scalar fields currently. | ||
| overrideGeneratedFieldType map[string]types.Type | ||
|
|
||
| // OverrideFieldNames allows to manually override the relevant field name to | ||
| // avoid possible Go struct name conflicts that may occur after Multiversion | ||
| // CRDs support. During field generation, there may be fields with the same | ||
|
|
@@ -1180,6 +1186,33 @@ func (r *Resource) RemoveSingletonListConversion(tfPath string) bool { | |
| return false | ||
| } | ||
|
|
||
| // OverrideScalarFieldType allows to manually override the type for the | ||
| // generated scalar field of a Resource at the specified Terraform path. | ||
| // The path is a Terraform field path without the wildcard segments, e.g., | ||
| // "x.y", even if "x" is a collection type. | ||
| // We only support overriding types for scalar fields as of now. | ||
| // Trying to override the type generated for a non-scalar path will result in | ||
| // a generation-time error. | ||
| func (r *Resource) OverrideScalarFieldType(path string, t types.Type) { | ||
| r.overrideGeneratedFieldType[path] = t | ||
| } | ||
|
|
||
| // FieldTypeOverrideConfiguration represents a configuration for a set of type | ||
| // overrides at a specific Terraform path. | ||
| type FieldTypeOverrideConfiguration struct { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One thing I thought about is exposing I didn't think the alternative for this but still wanted to discuss do we really want to export them for general use instead of really generation internal?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was initially thinking exposing the Currently, the only "intended" and exposed configuration API is the The cost of hiding the implementation detail from the provider authors is then we need to expose that configuration via an accessor because the code generation pipeline that consumes this configuration lives in the package |
||
| ParameterTypeOverride types.Type | ||
|
ulucinar marked this conversation as resolved.
|
||
| } | ||
|
|
||
| // FieldTypeOverride returns the type override configuration for the specified | ||
| // path. The path is a Terraform field path without the wildcard segments, | ||
| // e.g., "x.y", even if "x" is a collection type. | ||
| // Note: This accessor is meant to be only used by the code generator. | ||
| func (r *Resource) FieldTypeOverride(path string) FieldTypeOverrideConfiguration { | ||
| return FieldTypeOverrideConfiguration{ | ||
| ParameterTypeOverride: r.overrideGeneratedFieldType[path], | ||
| } | ||
| } | ||
|
|
||
| // SetEmbeddedObject sets the EmbeddedObject for the specified key. | ||
| // The key is a Terraform field path without the wildcard segments. | ||
| func (m SchemaElementOptions) SetEmbeddedObject(el string) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| // SPDX-FileCopyrightText: 2026 The Crossplane Authors <https://crossplane.io> | ||
| // | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| package internal | ||
|
|
||
| import ( | ||
| "encoding/json" | ||
| "fmt" | ||
|
|
||
| "github.com/crossplane/crossplane-runtime/v2/pkg/errors" | ||
| ) | ||
|
|
||
| const ( | ||
| errInvalidValue = "value must be a JSON string or %T, got %s" | ||
| ) | ||
|
|
||
| // Primitive is a type constraint that represents the supported primitive types | ||
| // for StringOrPrimitive. | ||
| type Primitive interface { | ||
| ~bool | | ||
| ~int | ~int8 | ~int16 | ~int32 | ~int64 | | ||
| ~uint | ~uint8 | ~uint16 | ~uint32 | ~uint64 | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| // StringOrPrimitive is stored canonically as a string, but can decode | ||
| // from other primitive values such as ints and bools. | ||
| type StringOrPrimitive[T Primitive] string | ||
|
|
||
| func (b *StringOrPrimitive[T]) UnmarshalJSON(data []byte) error { | ||
| // first try as a string value. | ||
| var s string | ||
| if err := json.Unmarshal(data, &s); err == nil { | ||
| *b = StringOrPrimitive[T](s) | ||
| return nil | ||
| } | ||
|
|
||
| // if not a string value, try as a value of the specified type parameter. | ||
| var v T | ||
| if err := json.Unmarshal(data, &v); err == nil { | ||
| *b = StringOrPrimitive[T](fmt.Sprint(v)) | ||
| return nil | ||
| } | ||
|
|
||
| return errors.Errorf(errInvalidValue, v, string(data)) | ||
| } | ||
|
|
||
| func (b *StringOrPrimitive[T]) MarshalJSON() ([]byte, error) { | ||
| // Always write back as string in the new canonical format. | ||
| return json.Marshal(*b) | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One thing that came to mind: should we validate that path actually exists in the Terraform schema? Right now a typo would silently become a no-op. It may never match anything during code generation. If we intentionally want to keep this like that, it might at least be worth documenting that invalid paths fail. What do you think?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I definitely agree we should do that. I considered implementing a validation in addition to the scalar type check we already have but the issue is the available canonical paths are not readily available to us. This is also the reason we don't have such checks for other similar configuration options like
config.Resource.AddSingletonListConversionand others. I was thinking we could try to address this cross-cutting validation concern in a future PR, because this already surfaced in this PR's discussions, let me check what we can do about it...