Skip to content

Commit c33cc57

Browse files
alexsomesanalexsomesan
authored
Handle 'x-kubernetes-preserve-unknown-fields' type annotation (#1646)
* Handle 'x-kubernetes-preserve-unknown-fields' type annotation in OpenAPI: changes to attributes of this type trigger whole resource recreation.
1 parent 8498cca commit c33cc57

File tree

13 files changed

+333
-29
lines changed

13 files changed

+333
-29
lines changed

‎manifest/const.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
package manifest
2+
3+
const (
4+
PreserveUnknownFieldsLabel string = "x-kubernetes-preserve-unknown-fields"
5+
)

‎manifest/morph/morph.go‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,3 +345,25 @@ func morphObjectToType(v tftypes.Value, t tftypes.Type, p *tftypes.AttributePath
345345
}
346346
return tftypes.Value{}, p.NewErrorf("[%s] unsupported morph of object value into type: %s", p.String(), t.String())
347347
}
348+
349+
// ValueToTypePath "normalizes" AttributePaths of values into a form that only describes the type hyerarchy.
350+
// this is used when comparing value paths to type hints generated during the translation from OpenAPI into tftypes.
351+
func ValueToTypePath(a *tftypes.AttributePath) *tftypes.AttributePath {
352+
if a == nil {
353+
return nil
354+
}
355+
ns := make([]tftypes.AttributePathStep, len(a.Steps()))
356+
os := a.Steps()
357+
for i := range os {
358+
switch os[i].(type) {
359+
case tftypes.AttributeName:
360+
ns[i] = tftypes.AttributeName(os[i].(tftypes.AttributeName))
361+
case tftypes.ElementKeyString:
362+
ns[i] = tftypes.ElementKeyString("#")
363+
case tftypes.ElementKeyInt:
364+
ns[i] = tftypes.ElementKeyInt(-1)
365+
}
366+
}
367+
368+
return tftypes.NewAttributePathWithSteps(ns)
369+
}

‎manifest/openapi/schema.go‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99

1010
"github.com/getkin/kin-openapi/openapi3"
1111
"github.com/hashicorp/terraform-plugin-go/tftypes"
12+
"github.com/hashicorp/terraform-provider-kubernetes/manifest"
1213
"github.com/mitchellh/hashstructure"
1314
)
1415

@@ -66,6 +67,20 @@ func getTypeFromSchema(elem *openapi3.Schema, stackdepth uint64, typeCache *sync
6667

6768
var t tftypes.Type
6869

70+
// Check if attribute type is tagged as 'x-kubernetes-preserve-unknown-fields' in OpenAPI.
71+
// If so, we add a type hint to indicate this and return DynamicPseudoType for this attribute,
72+
// since we have no further structural information about it.
73+
if xpufJSON, ok := elem.Extensions[manifest.PreserveUnknownFieldsLabel]; ok {
74+
var xpuf bool
75+
v, err := xpufJSON.(json.RawMessage).MarshalJSON()
76+
if err == nil {
77+
err = json.Unmarshal(v, &xpuf)
78+
if err == nil && xpuf {
79+
th[ap.String()] = manifest.PreserveUnknownFieldsLabel
80+
}
81+
}
82+
}
83+
6984
// check if type is in cache
7085
// HACK: this is temporarily disabled to diagnose a cache corruption issue.
7186
// if herr == nil {

‎manifest/payload/from_value.go‎

Lines changed: 2 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"strconv"
66

77
"github.com/hashicorp/terraform-plugin-go/tftypes"
8+
"github.com/hashicorp/terraform-provider-kubernetes/manifest/morph"
89
)
910

1011
// FromTFValue converts a Terraform specific tftypes.Value type object
@@ -46,7 +47,7 @@ func FromTFValue(in tftypes.Value, th map[string]string, ap *tftypes.AttributePa
4647
if err != nil {
4748
return nil, ap.NewErrorf("[%s] cannot extract contents of attribute: %s", ap.String(), err)
4849
}
49-
tp := valueToTypePath(ap)
50+
tp := morph.ValueToTypePath(ap)
5051
ot, ok := th[tp.String()]
5152
if ok && ot == "io.k8s.apimachinery.pkg.util.intstr.IntOrString" {
5253
n, err := strconv.Atoi(sv)
@@ -105,25 +106,3 @@ func FromTFValue(in tftypes.Value, th map[string]string, ap *tftypes.AttributePa
105106
return nil, ap.NewErrorf("[%s] cannot convert value of unknown type (%s)", ap.String(), in.Type().String())
106107
}
107108
}
108-
109-
// valueToTypePath "normalizes" AttributePaths of values into a form that only describes the type hyerarchy.
110-
// this is used when comparing value paths to type hints generated during the translation from OpenAPI into tftypes.
111-
func valueToTypePath(a *tftypes.AttributePath) *tftypes.AttributePath {
112-
if a == nil {
113-
return nil
114-
}
115-
ns := make([]tftypes.AttributePathStep, len(a.Steps()))
116-
os := a.Steps()
117-
for i := range os {
118-
switch os[i].(type) {
119-
case tftypes.AttributeName:
120-
ns[i] = tftypes.AttributeName(os[i].(tftypes.AttributeName))
121-
case tftypes.ElementKeyString:
122-
ns[i] = tftypes.ElementKeyString("#")
123-
case tftypes.ElementKeyInt:
124-
ns[i] = tftypes.ElementKeyInt(-1)
125-
}
126-
}
127-
128-
return tftypes.NewAttributePathWithSteps(ns)
129-
}

‎manifest/payload/from_value_test.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"testing"
77

88
"github.com/hashicorp/terraform-plugin-go/tftypes"
9+
"github.com/hashicorp/terraform-provider-kubernetes/manifest/morph"
910
)
1011

1112
func TestFromTFValue(t *testing.T) {
@@ -176,7 +177,7 @@ func TestValueToTypePath(t *testing.T) {
176177
}
177178
for n, s := range samples {
178179
t.Run(n, func(t *testing.T) {
179-
p := valueToTypePath(s.In)
180+
p := morph.ValueToTypePath(s.In)
180181
if !p.Equal(s.Out) {
181182
t.Logf("Expected %#v, received: %#v", s.Out, p)
182183
t.Fail()

‎manifest/payload/to_value.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"strconv"
77

88
"github.com/hashicorp/terraform-plugin-go/tftypes"
9+
"github.com/hashicorp/terraform-provider-kubernetes/manifest/morph"
910
)
1011

1112
// ToTFValue converts a Kubernetes dynamic client unstructured value
@@ -51,7 +52,7 @@ func ToTFValue(in interface{}, st tftypes.Type, th map[string]string, at *tftype
5152
case st.Is(tftypes.Number) || st.Is(tftypes.DynamicPseudoType):
5253
return tftypes.NewValue(tftypes.Number, new(big.Float).SetInt64(int64(in.(int)))), nil
5354
case st.Is(tftypes.String):
54-
ht, ok := th[valueToTypePath(at).String()]
55+
ht, ok := th[morph.ValueToTypePath(at).String()]
5556
if ok && ht == "io.k8s.apimachinery.pkg.util.intstr.IntOrString" { // We store this in state as "string"
5657
return tftypes.NewValue(tftypes.String, strconv.FormatInt(int64(in.(int)), 10)), nil
5758
}
@@ -64,7 +65,7 @@ func ToTFValue(in interface{}, st tftypes.Type, th map[string]string, at *tftype
6465
case st.Is(tftypes.Number) || st.Is(tftypes.DynamicPseudoType):
6566
return tftypes.NewValue(tftypes.Number, new(big.Float).SetInt64(in.(int64))), nil
6667
case st.Is(tftypes.String):
67-
ht, ok := th[valueToTypePath(at).String()]
68+
ht, ok := th[morph.ValueToTypePath(at).String()]
6869
if ok && ht == "io.k8s.apimachinery.pkg.util.intstr.IntOrString" { // We store this in state as "string"
6970
return tftypes.NewValue(tftypes.String, strconv.FormatInt(in.(int64), 10)), nil
7071
}
@@ -77,7 +78,7 @@ func ToTFValue(in interface{}, st tftypes.Type, th map[string]string, at *tftype
7778
case st.Is(tftypes.Number) || st.Is(tftypes.DynamicPseudoType):
7879
return tftypes.NewValue(tftypes.Number, new(big.Float).SetInt64(int64(in.(int32)))), nil
7980
case st.Is(tftypes.String):
80-
ht, ok := th[valueToTypePath(at).String()]
81+
ht, ok := th[morph.ValueToTypePath(at).String()]
8182
if ok && ht == "io.k8s.apimachinery.pkg.util.intstr.IntOrString" { // We store this in state as "string"
8283
return tftypes.NewValue(tftypes.String, strconv.FormatInt(int64(in.(int32)), 10)), nil
8384
}
@@ -90,7 +91,7 @@ func ToTFValue(in interface{}, st tftypes.Type, th map[string]string, at *tftype
9091
case st.Is(tftypes.Number) || st.Is(tftypes.DynamicPseudoType):
9192
return tftypes.NewValue(tftypes.Number, new(big.Float).SetInt64(int64(in.(int16)))), nil
9293
case st.Is(tftypes.String):
93-
ht, ok := th[valueToTypePath(at).String()]
94+
ht, ok := th[morph.ValueToTypePath(at).String()]
9495
if ok && ht == "io.k8s.apimachinery.pkg.util.intstr.IntOrString" { // We store this in state as "string"
9596
return tftypes.NewValue(tftypes.String, strconv.FormatInt(int64(in.(int16)), 10)), nil
9697
}

‎manifest/provider/plan.go‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66

77
"github.com/hashicorp/terraform-plugin-go/tfprotov5"
88
"github.com/hashicorp/terraform-plugin-go/tftypes"
9+
"github.com/hashicorp/terraform-provider-kubernetes/manifest"
910
"github.com/hashicorp/terraform-provider-kubernetes/manifest/morph"
1011
"github.com/hashicorp/terraform-provider-kubernetes/manifest/payload"
1112
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -277,7 +278,7 @@ func (s *RawProviderServer) PlanResourceChange(ctx context.Context, req *tfproto
277278
}
278279

279280
// Request a complete type for the resource from the OpenAPI spec
280-
objectType, _, err := s.TFTypeFromOpenAPI(ctx, gvk, false)
281+
objectType, hints, err := s.TFTypeFromOpenAPI(ctx, gvk, false)
281282
if err != nil {
282283
return resp, fmt.Errorf("failed to determine resource type ID: %s", err)
283284
}
@@ -403,6 +404,13 @@ func (s *RawProviderServer) PlanResourceChange(ctx context.Context, req *tfproto
403404
}
404405
nowCfg, restPath, err := tftypes.WalkAttributePath(ppMan, ap)
405406
hasChanged = err == nil && len(restPath.Steps()) == 0 && wasCfg.(tftypes.Value).IsKnown() && !wasCfg.(tftypes.Value).Equal(nowCfg.(tftypes.Value))
407+
if hasChanged {
408+
h, ok := hints[morph.ValueToTypePath(ap).String()]
409+
if ok && h == manifest.PreserveUnknownFieldsLabel {
410+
apm := append(tftypes.NewAttributePath().WithAttributeName("manifest").Steps(), ap.Steps()...)
411+
resp.RequiresReplace = append(resp.RequiresReplace, tftypes.NewAttributePathWithSteps(apm))
412+
}
413+
}
406414
if isComputed {
407415
if hasChanged {
408416
return tftypes.NewValue(v.Type(), tftypes.UnknownValue), nil

‎manifest/test/acceptance/customresource_x_preserve_unknown_fields_test.go‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
//go:build acceptance
2+
// +build acceptance
3+
4+
package acceptance
5+
6+
import (
7+
"context"
8+
"encoding/json"
9+
"fmt"
10+
"strings"
11+
"testing"
12+
"time"
13+
14+
"github.com/hashicorp/go-hclog"
15+
"github.com/hashicorp/terraform-provider-kubernetes/manifest/provider"
16+
tfstatehelper "github.com/hashicorp/terraform-provider-kubernetes/manifest/test/helper/state"
17+
)
18+
19+
func TestKubernetesManifest_CustomResource_x_preserve_unknown_fields(t *testing.T) {
20+
ctx := context.Background()
21+
22+
reattachInfo, err := provider.ServeTest(ctx, hclog.Default(), t)
23+
if err != nil {
24+
t.Errorf("Failed to create provider instance: %q", err)
25+
}
26+
27+
kind := strings.Title(randString(8))
28+
plural := strings.ToLower(kind) + "s"
29+
group := "terraform.io"
30+
version := "v1"
31+
groupVersion := group + "/" + version
32+
crd := fmt.Sprintf("%s.%s", plural, group)
33+
34+
name := strings.ToLower(randName())
35+
namespace := "default" //randName()
36+
37+
tfvars := TFVARS{
38+
"name": name,
39+
"namespace": namespace,
40+
"kind": kind,
41+
"plural": plural,
42+
"group": group,
43+
"group_version": groupVersion,
44+
"cr_version": version,
45+
}
46+
47+
crdStep := tfhelper.RequireNewWorkingDir(ctx, t)
48+
crdStep.SetReattachInfo(ctx, reattachInfo)
49+
defer func() {
50+
crdStep.Destroy(ctx)
51+
crdStep.Close()
52+
k8shelper.AssertResourceDoesNotExist(t, "apiextensions.k8s.io/v1", "customresourcedefinitions", crd)
53+
}()
54+
55+
tfconfig := loadTerraformConfig(t, "x-kubernetes-preserve-unknown-fields/crd/test.tf", tfvars)
56+
crdStep.SetConfig(ctx, string(tfconfig))
57+
crdStep.Init(ctx)
58+
crdStep.Apply(ctx)
59+
k8shelper.AssertResourceExists(t, "apiextensions.k8s.io/v1", "customresourcedefinitions", crd)
60+
61+
// wait for API to finish ingesting the CRD
62+
time.Sleep(5 * time.Second) //lintignore:R018
63+
64+
reattachInfo2, err := provider.ServeTest(ctx, hclog.Default(), t)
65+
if err != nil {
66+
t.Errorf("Failed to create additional provider instance: %q", err)
67+
}
68+
69+
step1 := tfhelper.RequireNewWorkingDir(ctx, t)
70+
step1.SetReattachInfo(ctx, reattachInfo2)
71+
defer func() {
72+
step1.Destroy(ctx)
73+
step1.Close()
74+
k8shelper.AssertResourceDoesNotExist(t, groupVersion, kind, name)
75+
}()
76+
77+
tfconfig = loadTerraformConfig(t, "x-kubernetes-preserve-unknown-fields/test-cr-1.tf", tfvars)
78+
step1.SetConfig(ctx, string(tfconfig))
79+
step1.Init(ctx)
80+
step1.Apply(ctx)
81+
82+
s1, err := step1.State(ctx)
83+
if err != nil {
84+
t.Fatalf("Failed to retrieve terraform state: %q", err)
85+
}
86+
tfstate := tfstatehelper.NewHelper(s1)
87+
tfstate.AssertAttributeValues(t, tfstatehelper.AttributeValues{
88+
"kubernetes_manifest.test.object.metadata.name": name,
89+
"kubernetes_manifest.test.object.metadata.namespace": namespace,
90+
"kubernetes_manifest.test.object.spec.count": json.Number("100"),
91+
"kubernetes_manifest.test.object.spec.resources": map[string]interface{}{
92+
"foo": interface{}("bar"),
93+
},
94+
})
95+
96+
tfconfig = loadTerraformConfig(t, "x-kubernetes-preserve-unknown-fields/test-cr-2.tf", tfvars)
97+
step1.SetConfig(ctx, string(tfconfig))
98+
step1.Apply(ctx)
99+
100+
s2, err := step1.State(ctx)
101+
if err != nil {
102+
t.Fatalf("Failed to retrieve terraform state: %q", err)
103+
}
104+
tfstate2 := tfstatehelper.NewHelper(s2)
105+
tfstate2.AssertAttributeValues(t, tfstatehelper.AttributeValues{
106+
"kubernetes_manifest.test.object.metadata.name": name,
107+
"kubernetes_manifest.test.object.metadata.namespace": namespace,
108+
"kubernetes_manifest.test.object.spec.count": json.Number("100"),
109+
"kubernetes_manifest.test.object.spec.resources": map[string]interface{}{
110+
"foo": interface{}("bar"),
111+
"baz": interface{}("42"),
112+
},
113+
})
114+
}

‎manifest/test/acceptance/testdata/x-kubernetes-preserve-unknown-fields/crd/test.tf‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
resource "kubernetes_manifest" "customresourcedefinition_cephrbdmirrors_ceph_rook_io" {
2+
manifest = {
3+
apiVersion = "apiextensions.k8s.io/v1"
4+
kind = "CustomResourceDefinition"
5+
metadata = {
6+
name = "${var.plural}.${var.group}"
7+
}
8+
spec = {
9+
group = var.group
10+
names = {
11+
kind = var.kind
12+
plural = var.plural
13+
}
14+
scope = "Namespaced"
15+
versions = [
16+
{
17+
name = var.cr_version
18+
schema = {
19+
openAPIV3Schema = {
20+
properties = {
21+
spec = {
22+
properties = {
23+
annotations = {
24+
nullable = true
25+
type = "object"
26+
"x-kubernetes-preserve-unknown-fields" = true
27+
}
28+
count = {
29+
maximum = 100
30+
minimum = 1
31+
type = "integer"
32+
}
33+
peers = {
34+
properties = {
35+
secretNames = {
36+
items = {
37+
type = "string"
38+
}
39+
type = "array"
40+
}
41+
}
42+
type = "object"
43+
}
44+
placement = {
45+
nullable = true
46+
type = "object"
47+
"x-kubernetes-preserve-unknown-fields" = true
48+
}
49+
priorityClassName = {
50+
type = "string"
51+
}
52+
resources = {
53+
nullable = true
54+
type = "object"
55+
"x-kubernetes-preserve-unknown-fields" = true
56+
}
57+
}
58+
type = "object"
59+
}
60+
status = {
61+
type = "object"
62+
"x-kubernetes-preserve-unknown-fields" = true
63+
}
64+
}
65+
type = "object"
66+
}
67+
}
68+
served = true
69+
storage = true
70+
subresources = {
71+
status = {}
72+
}
73+
},
74+
]
75+
}
76+
}
77+
}

0 commit comments

Comments
 (0)