Skip to content
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

🐛 Fix the wait conditions issue in the kubernetes_manifest resource #2173

Merged
merged 6 commits into from
Jul 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/2173.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
`resource/kubernetes_manifest`: fix an issue with the `kubernetes_manifest` resource, where an object fails to update correctly when employing wait conditions and thus some attributes are not available for the reference after creation.
```
6 changes: 5 additions & 1 deletion kubernetes/test-infra/gke/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ variable "enable_alpha" {
default = false
}

variable "cluster_name" {
default = ""
}

data "google_compute_zones" "available" {
}

Expand All @@ -36,7 +40,7 @@ resource "google_service_account" "default" {

resource "google_container_cluster" "primary" {
provider = google-beta
name = "tf-acc-test-${random_id.cluster_name.hex}"
name = var.cluster_name != "" ? var.cluster_name : "tf-acc-test-${random_id.cluster_name.hex}"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change is not related to the fix directly, however, I found this useful to configure the cluster name for manual testing purposes while working on this issue.

location = data.google_compute_zones.available.names[0]
node_version = data.google_container_engine_versions.supported.latest_node_version
min_master_version = data.google_container_engine_versions.supported.latest_master_version
Expand Down
38 changes: 26 additions & 12 deletions manifest/provider/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -404,18 +404,6 @@ func (s *RawProviderServer) ApplyResourceChange(ctx context.Context, req *tfprot
return resp, nil
}

newResObject, err := payload.ToTFValue(RemoveServerSideFields(result.Object), tsch, th, tftypes.NewAttributePath())
if err != nil {
resp.Diagnostics = append(resp.Diagnostics,
&tfprotov5.Diagnostic{
Severity: tfprotov5.DiagnosticSeverityError,
Summary: "Conversion from Unstructured to tftypes.Value failed",
Detail: err.Error(),
})
return resp, nil
}
s.logger.Trace("[ApplyResourceChange][Apply]", "[payload.ToTFValue]", dump(newResObject))

wt, _, err := s.TFTypeFromOpenAPI(ctx, gvk, true)
if err != nil {
return resp, fmt.Errorf("failed to determine resource type ID: %s", err)
Expand Down Expand Up @@ -454,8 +442,34 @@ func (s *RawProviderServer) ApplyResourceChange(ctx context.Context, req *tfprot
}
return resp, nil
}

r, err := rs.Get(ctxDeadline, rname, metav1.GetOptions{})
if err != nil {
s.logger.Error("[ApplyResourceChange][ReadAfterWait]", "API error", dump(err), "API response", dump(result))
resp.Diagnostics = append(resp.Diagnostics,
&tfprotov5.Diagnostic{
Severity: tfprotov5.DiagnosticSeverityError,
Summary: fmt.Sprintf(`Failed to read resource %q after wait conditions`, rname),
Detail: err.Error(),
})

return resp, nil
}
result = r
}

newResObject, err := payload.ToTFValue(RemoveServerSideFields(result.Object), tsch, th, tftypes.NewAttributePath())
if err != nil {
resp.Diagnostics = append(resp.Diagnostics,
&tfprotov5.Diagnostic{
Severity: tfprotov5.DiagnosticSeverityError,
Summary: "Conversion from Unstructured to tftypes.Value failed",
Detail: err.Error(),
})
return resp, nil
}
s.logger.Trace("[ApplyResourceChange][Apply]", "[payload.ToTFValue]", dump(newResObject))

compObj, err := morph.DeepUnknown(tsch, newResObject, tftypes.NewAttributePath())
if err != nil {
return resp, err
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# Copyright (c) HashiCorp, Inc.
# SPDX-License-Identifier: MPL-2.0

resource "kubernetes_manifest" "test" {
manifest = {
apiVersion = "v1"
kind = "Secret"
metadata = {
name = var.name
namespace = var.namespace

annotations = {
"kubernetes.io/service-account.name" = "default"
}
}
type = "kubernetes.io/service-account-token"
}
wait {
fields = {
"metadata.annotations[\"kubernetes.io/service-account.uid\"]" = "^.*$",
}
}

timeouts {
create = "10s"
}
}

output "test" {
value = kubernetes_manifest.test.object.metadata.annotations["kubernetes.io/service-account.uid"]
}
48 changes: 48 additions & 0 deletions manifest/test/acceptance/wait_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,3 +223,51 @@ func TestKubernetesManifest_Wait_InvalidCondition(t *testing.T) {
t.Fatalf("Waiter should have timed out")
}
}

func TestKubernetesManifest_WaitFields_Annotations(t *testing.T) {
ctx := context.Background()

name := randName()
namespace := randName()

reattachInfo, err := provider.ServeTest(ctx, hclog.Default(), t)
if err != nil {
t.Errorf("Failed to create provider instance: %q", err)
}

tf := tfhelper.RequireNewWorkingDir(ctx, t)
tf.SetReattachInfo(ctx, reattachInfo)
defer func() {
tf.Destroy(ctx)
tf.Close()
k8shelper.AssertNamespacedResourceDoesNotExist(t, "v1", "secrets", namespace, name)
}()

k8shelper.CreateNamespace(t, namespace)
defer k8shelper.DeleteResource(t, namespace, kubernetes.NewGroupVersionResource("v1", "namespaces"))

tfvars := TFVARS{
"namespace": namespace,
"name": name,
}
tfconfig := loadTerraformConfig(t, "Wait/wait_for_fields_annotations.tf", tfvars)
tf.SetConfig(ctx, tfconfig)
tf.Init(ctx)

tf.Apply(ctx)

k8shelper.AssertNamespacedResourceExists(t, "v1", "secrets", namespace, name)

st, err := tf.State(ctx)
if err != nil {
t.Fatalf("Failed to obtain state: %q", err)
}
tfstate := tfstatehelper.NewHelper(st)
tfstate.AssertAttributeValues(t, tfstatehelper.AttributeValues{
"kubernetes_manifest.test.wait.0.fields": map[string]interface{}{
"metadata.annotations[\"kubernetes.io/service-account.uid\"]": "^.*$",
},
})

tfstate.AssertOutputExists(t, "test")
}
30 changes: 30 additions & 0 deletions manifest/test/helper/state/state_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,17 @@ func getAttributesValuesFromResource(state *Helper, address string) (interface{}
return nil, fmt.Errorf("Could not find resource %q in state", address)
}

// getOutputValues gets the given output name value from the state
func getOutputValues(state *Helper, name string) (interface{}, error) {
for n, v := range state.Values.Outputs {
if n == name {
return v.Value, nil
}
}

return nil, fmt.Errorf("Could not find output %q in state", name)
}

var errFieldNotFound = fmt.Errorf("Field not found")

// findAttributeValue will return the value of the attribute at the given address in a tree of arrays and maps
Expand Down Expand Up @@ -107,6 +118,18 @@ func (s *Helper) GetAttributeValue(t *testing.T, address string) interface{} {
return value
}

// GetOutputValue gets the given output name value from the state
func (s *Helper) GetOutputValue(t *testing.T, name string) interface{} {
t.Helper()

value, err := getOutputValues(s, name)
if err != nil {
t.Fatal(err)
}

return value
}

// AttributeValues is a convenience type for supplying maps of attributes and values
// to AssertAttributeValues
type AttributeValues map[string]interface{}
Expand Down Expand Up @@ -207,3 +230,10 @@ func (s *Helper) AssertAttributeFalse(t *testing.T, address string) {
assert.False(t, v, fmt.Sprintf("Address: %q", address))
}
}

// AssertOutputExists will fail the test if the output does not exist
func (s *Helper) AssertOutputExists(t *testing.T, name string) {
t.Helper()

s.GetOutputValue(t, name)
}