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

[Bug]: kyverno_io_cluster_policy_v1_manifest - spec.rules.context.api_call.data.value is a map of strings #169

Closed
calvinbui opened this issue Jul 18, 2024 · 5 comments · Fixed by #193
Assignees
Labels
bug Something isn't working

Comments

@calvinbui
Copy link

Relevant Terraform configuration

{
  name = "response"
  api_call = {
    method = "POST"

    data = [
      {
        key   = "images"
        value = "{{images}}"
      },
    ]
  }
}

Relevant log output

Inappropriate value for attribute "spec": attribute "rules": element 0:
attribute "context": element 1: attribute "api_call": attribute "data":
element 0: attribute "value": map of string required.

Operation failed: failed running terraform plan (exit 1)
@calvinbui calvinbui added the bug Something isn't working label Jul 18, 2024
@calvinbui
Copy link
Author

The issue is that spec.rules.context.api_call.data.value is expected to be a map of strings.

However, with how Kyverno has coded it, it can be a string, a list or a map: https://github.com/kyverno/kyverno/blob/279895c60056b1552476663b0fa814cb0e7d7597/api/kyverno/v1/common_types.go#L235

Simply, a string is passed and then converted using the JSON module.

Sample code, you can replace {"service": "something"} with a bool, int, string, etc.

package main

import (
	"encoding/json"
	"fmt"

	apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
)

type RequestData struct {
	// Key is a unique identifier for the data value
	Key string `json:"key" yaml:"key"`

	// Value is the data value
	Value *apiextv1.JSON `json:"value" yaml:"value"`
}

func main() {
	data := []RequestData{
		{
			Key: "imageReferences",
			Value: &apiextv1.JSON{
				Raw: []byte(`{"service": "something"}`), // Notice the backticks for a valid JSON string
			},
		},
	}

	dataMap := make(map[string]interface{})
	for _, d := range data {
		var value interface{}
		if err := json.Unmarshal(d.Value.Raw, &value); err != nil {
			fmt.Printf("Error unmarshaling JSON for key %s: %v\n", d.Key, err)
			continue
		}
		dataMap[d.Key] = value
	}

	for key, value := range dataMap {
		fmt.Printf("%s: %v\n", key, value)
	}
}

I'm not sure what the best solution is.

@calvinbui
Copy link
Author

The CRD however is an object:

❯ kubectl explain clusterpolicies.spec.rules.context.apiCall.data.value
GROUP:      kyverno.io
KIND:       ClusterPolicy
VERSION:    v1

FIELD: value <Object>

DESCRIPTION:
    Value is the data value

Other examples from Kyverno where it is a string and a map

@sebhoss
Copy link
Member

sebhoss commented Sep 12, 2024

hey @calvinbui sorry for the super late reply & thanks for opening this ticket

The complete failing example looks like this:

data "k8s_kyverno_io_cluster_policy_v1_manifest" "example" {
  metadata = {
    name = "some-name"
  }
  spec = {
    rules = [
      {
        name = "some-rule"
        context = [
          {
            name = "response"
            api_call = {
              method = "POST"
              data = [
                {
                  key   = "images"
                  value = "{{images}}"
                },
              ]
            }
          }
        ]
      }
    ]
  }
}

Technically, this is a prime example for dynamic types in Terraform, however due to hashicorp/terraform-plugin-framework#973 we cannot use dynamic types in collections. Conceptually this is similar to hashicorp/terraform-plugin-codegen-openapi#82 and I'm hoping that hashicorp/terraform-plugin-codegen-framework#132 can be used as some kind of blue print for other providers on how to deal with dynamic types in these situations.

Since all of that is still open, I think the best we can do for now is to enhance the code generator in this provider to allow types to be overwritten, then declare the value field as a normalized JSON type from https://github.com/hashicorp/terraform-plugin-framework-jsontypes and finally help out a little with the YAML serialization, so that the final rendered output looks as expected.

A valid config block would look like this:

data "k8s_kyverno_io_cluster_policy_v1_manifest" "example" {
  metadata = {
    name = "some-name"
  }
  spec = {
    rules = [
      {
        name = "some-rule"
        context = [
          {
            name = "response"
            api_call = {
              method = "POST"
              data = [
                {
                  key   = "images"
                  value = <<EOT
                    "{{images}}"
                  EOT
                },
              ]
            }
          }
        ]
      }
    ]
  }
}

Everything between EOT can be any valid JSON.

@sebhoss
Copy link
Member

sebhoss commented Sep 12, 2024

Ok an implementation is in #193 and that will be released on monday. The HCL looks slightly different compared to my last comment:

data "k8s_kyverno_io_cluster_policy_v1_manifest" "example" {
  metadata = {
    name = "some-name"
  }
  spec = {
    rules = [
      {
        name = "some-rule"
        context = [
          {
            name = "response"
            api_call = {
              method = "POST"
              data = [
                {
                  key   = "images"
                  value = jsonencode("{{images}}") # <-- use jsonencode to wrap any JSON value
                },
              ]
            }
          }
        ]
      }
    ]
  }
}

You don't strictly have to use jsonencode but I think it's safer than just writing any string into an heredoc. I've added several examples to the k8s_kyverno_io_cluster_policy_v1_manifest resource and use those as test cases as well. The JSON value is normalized with that normalized JSON type from https://github.com/hashicorp/terraform-plugin-framework-jsontypes and I've opened hashicorp/terraform-plugin-framework-jsontypes#75 because I'm currently copying some code from their repo but would love not to do that in the future.

@sebhoss
Copy link
Member

sebhoss commented Sep 18, 2024

Not entirely sure why the automated release workflow did not even start but I just triggered it manually and the new version is live. See https://registry.terraform.io/providers/metio/k8s/latest/docs/data-sources/kyverno_io_cluster_policy_v1_manifest for docs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants