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

Feature: Custom Resource #286

Closed
wants to merge 4 commits into from

Conversation

timthesinner
Copy link

@timthesinner timthesinner commented Jan 25, 2019

Looking to get an assessment on where I am headed with this. Resource naming, constructs, ETC. The biggest architectural change is that I converted to using interfaces when interacting with K8S instead of a formal type. I needed to extend the base kubernetes.Clientset in order to enable a custom resource client.

Resource state management

We need to handle state diff between K8S, TF, and Desired. This was accomplished by adding transient fields to the computed spec field dynamically from a custom diff function. Take a look at the diff function here. Here are what each of the transient fields mean:

  • transient-delete is a field that will get removed (may have been added out of band)
  • transient-add is a field that will get added (may have been deleted out of band)
  • transient-update is a field that will get replaced (may have been modified out of band)

Notes:

  • Using the flattened dict and keyed prefix is real clean when building the PATCH options.
    • path is replace the key prefix with /spec/ and replace all . with /
    • value is spec[key - prefix]
    • op is the prefix type (a constant)
  • We end up writing the spec from the returned response from K8S so these transient fields are never persisted into tfstate.
  • The diff accounts for updates to the yaml field
  • Updating K8S by hand, and then updating yaml to match will result in a delta to the yaml field, but no update ops will be sent to K8S.

I have been doing all my testing with a terraform that creates a ClusterIssuer using the yaml template below.

resource "kubernetes_generic" "test" {
    api_version = "certmanager.k8s.io/v1alpha1"
    kind ="ClusterIssuer"

    metadata {
        name = "terraform-example"
    }

    yaml = <<EOF
acme:
    email: email
    server: server
    privateKeySecretRef:
        name: secret-dev2
    dns01:
        providers:
        - name: route53
        route53:
            region: region
            hostedZoneID: zoneId
            accessKeyID: accessKey
            secretAccessKeySecretRef:
                name: secret
                key: secret-access-key
EOF
}

#215

@bukzor
Copy link

bukzor commented Feb 5, 2019

What's the state of this branch? Is it still as described in OP (no updates)?

I'm very excited as a potential user of something like this. Would you advise against attempting to use this branch for "real work"?

@mmalecki
Copy link

mmalecki commented Feb 5, 2019

This feels similar to #195, which resulted in a separate plugin being created: https://github.com/lawrencegripper/terraform-provider-kubernetes-yaml

@timthesinner
Copy link
Author

I was looking for initial feedback on naming conventions ETC, didn't hear anything so its been on the back burner for me.

The branch needs tests and I need to work through the diff/update logic to conform to the JSONPatch format that the API expects.

If I get some time tomorrow I can wrap up that last bit and it will be fully functional minus tests.

@yacut
Copy link

yacut commented Feb 17, 2019

@timthesinner any update on this?

Notes:
 - Created an ExtendedClientset allowing generic access to K8S REST client
 - Refactored all usage of the provider to the K8S interfaces
 - Added a dependency on "github.com/ghodss/yaml"
 - Handle create, update, delete for custom resources
 - Custom resource update state delta between K8S, TF, and Desired is handled through transient fields
@timthesinner
Copy link
Author

timthesinner commented Feb 18, 2019

@yacut, @bukzor at this point create, edit, and delete are all working. Out of band changes to K8S are also handled with a custom diff routine. When I say handled, whatever is configured in the yaml portion of the resource is the resulting state in K8S. If you were to add, delete, or modify any portion of the resource spec manually in K8S those modifications will all be replaced on terraform apply.

@alexsomesan As the primary on this feature for TF 0.12 could you take a look at this, I think I got a decent UX between TF/K8S/Desired states using a transient key in the state map.

Resource state management

Note on the transient fields, these are added into the spec dynamically from a custom diff function. We end up writing the spec from the returned response from K8S so these transient fields are never persisted into tfstate. This transient mapping is necessary, if we just rely on the internal state of the spec we cannot reliably diff between K8S state, TF state, and desired state. Here are what each of the transient fields mean:

  • transient-delete is a field that was configured out of band from TF, it will get removed
  • transient-add is a field that will get added (may have been deleted out of band)
  • transient-update is a field that will get replaced (may have been modified out of band)

Example workflow:

After initial application of the terraform resource. It was manually edited in K8S, fields were added, deleted and modified. This resource is exclusively managed by terraform, those modifications will be replaced (and noted in the terraform plan/apply output.

Edited resource

After I applied the TF configuration I manually updated the resource, this is the output of $ kubectl get clusterissuer terraform-example -o yaml

kind: ClusterIssuer
metadata:
  annotations:
    kubectl.kubernetes.io/last-applied-configuration: |
      {"apiVersion":"certmanager.k8s.io/v1alpha1","kind":"ClusterIssuer","metadata":{"annotations":{},"name":"terraform-example"},"spec":{"acme":{"dns01":{"providers":[{"name":"route59"}],"route53":{"region":"region","secretAccessKeySecretRef":{"key":"secret-access-key","name":"secret"}}},"email":"email","privateKeySecretRef":{"name":"secret-dev3"},"server":"server","server2":"test"}}}
  clusterName: ""
  creationTimestamp: 2019-02-17T23:49:06Z
  generation: 0
  name: terraform-example
  namespace: ""
  resourceVersion: "63297229"
  selfLink: /apis/certmanager.k8s.io/v1alpha1/terraform-example
  uid: 9c94369d-330e-11e9-b5ef-0e204540d6a2
spec:
  acme:
    dns01:
      providers:
      - name: route59
      route53:
        region: region
        secretAccessKeySecretRef:
          key: secret-access-key
          name: secret
    email: email
    privateKeySecretRef:
      name: secret-dev3
    server: server
    server2: test

Manage resource using TF (revert changes)

After running terraform apply, because I did not modify the original main.tf the delta reported here is not in the yaml field its in the computed spec field. If I had also modified the main.tf that delta would also have been presented.

kubernetes_generic.test: Refreshing state... (ID: certmanager.k8s.io/v1alpha1/terraform-example)

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  ~ kubernetes_generic.test
      spec.%:                                               "10" => "15"
      spec.transitent-add.acme.dns01.route53.accessKeyID:   "" => "accessKey"
      spec.transitent-add.acme.dns01.route53.hostedZoneID:  "" => "zoneId"
      spec.transitent-delete.acme.server2:                  "" => "'test' --> ''"
      spec.transitent-update.acme.dns01.providers.0.name:   "" => "'route59' --> 'route53'"
      spec.transitent-update.acme.privateKeySecretRef.name: "" => "'secret-dev3' --> 'secret-dev-test'"


Plan: 0 to add, 1 to change, 0 to destroy.

@Tazer
Copy link

Tazer commented Feb 21, 2019

@timthesinner just tried running your branch :) but go into a "loop"

Cause first it complained about
kubernetes_generic.cloudflare-issuer: Issuer.certmanager.k8s.io "cloudflare-issuer" is invalid: metadata.namespace: Required value

Code looks like this

resource "kubernetes_generic" "cloudflare-issuer" {
    api_version = "certmanager.k8s.io/v1alpha1"
    kind ="Issuer"

    metadata {
        name = "cloudflare-issuer"
    }

    yaml = <<EOF
  acme:
    email: xxx
    server: https://acme-staging-v02.api.letsencrypt.org/directory
    privateKeySecretRef:
      name: xxx
    dns01:
      providers:
        - name: prod-clouddns
          cloudflare:
            email: xxx
            apiKeySecretRef:
              name: xxx
              key: api-key
EOF
}

Then when I added namespace property I got Error: kubernetes_generic.cloudflare-issuer: metadata.0: invalid or unknown key: namespace

Config:

resource "kubernetes_generic" "cloudflare-issuer" {
    api_version = "certmanager.k8s.io/v1alpha1"
    kind ="Issuer"

    metadata {
        name = "cloudflare-issuer"
        namespace = "default"
    }

    yaml = <<EOF
  acme:
    email: xxx
    server: https://acme-staging-v02.api.letsencrypt.org/directory
    privateKeySecretRef:
      name: xxx
    dns01:
      providers:
        - name: prod-clouddns
          cloudflare:
            email: xxx
            apiKeySecretRef:
              name: xxx
              key: api-key
EOF
}

@timthesinner
Copy link
Author

@Tazer good find, I forgot to test generic resources in namespaces. I added the field to the metadata schema and updated the create logic to support custom resources that belong in a namespace.

Please let me know if you run into any additional issues.

@Tazer
Copy link

Tazer commented Feb 25, 2019

@timthesinner tried some more today adding custom resources.

Got into "unexpected EOF"

my terraform resource

resource "kubernetes_generic" "cr-clusterissuer" {
  api_version = "apiextensions.k8s.io/v1beta1"
  kind        = "CustomResourceDefinition"

  metadata {
    name = "clusterissuers.certmanager.k8s.io"

    labels {
      app = "cert-manager"
    }
  }

  yaml = <<EOF
  group: certmanager.k8s.io
  version: v1alpha1
  names:
    kind: ClusterIssuer
    plural: clusterissuers
  scope: Cluster
EOF
}

YAML file ( that I try to write with the kubernetes generic resource )

apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
  name: clusterissuers.certmanager.k8s.io
  labels:
    app: cert-manager
spec:
  group: certmanager.k8s.io
  version: v1alpha1
  names:
    kind: ClusterIssuer
    plural: clusterissuers
  scope: Cluster

Maybe I'm doing something wrong?

@timthesinner
Copy link
Author

@Tazer it looks like your trying to create a CustomResourceDefinition on K8S and that's failing (error messages are not clean just yet)probably because that CRD already exists for you.

The problem is coming from your kind/api_versions are:

  api_version = "apiextensions.k8s.io/v1beta1"
  kind        = "CustomResourceDefinition"

But they should be (looking at your body, looks like your trying to make a ClusterIssuer)

  api_version = "certmanager.k8s.io/v1alpha1"
  kind ="ClusterIssuer"

@Tazer
Copy link

Tazer commented Feb 25, 2019

@timthesinner think my files are correct and CRD's isn't existing. Cause I did run the code at first and got the "already exist" error , deleted the CRD and then got above error.

So what I'm trying to do is to create the definition of a Clusterissuer so I can create Clusterussuers later.

Basically it's from here:
https://github.com/helm/charts/blob/master/stable/cert-manager/README.md#installing-the-chart

So cert-manager requires you to create CRD's before installing the helm chart. and what you need to create is in this file https://raw.githubusercontent.com/jetstack/cert-manager/release-0.6/deploy/manifests/00-crds.yaml

( It works fine when I run via kubectl )

@timthesinner
Copy link
Author

timthesinner commented Feb 26, 2019

@Tazer interesting, the problem wasn't in the TF you posted, I assume you have other CRD in your TF. I was able to get an EOF by applying the certificates CRD from the install guide. Specifically I was running into issues with the additionalPrinterColumns field.

I fixed two problems in the latest commit:

  1. YAML parser treats numbers as floats and the flatten library panics when it sees a float.
  2. CRD spec allows additionalPrinterColumns on POST but it doesn't seem to allow additionalPrinterColumns on PATCH. This is really concerning, this feature will be non supportable if this list starts growing.

Here is the TF that I was able to reproduce an EOF with

resource "kubernetes_generic" "certificates" {
    api_version = "apiextensions.k8s.io/v1beta1"
    kind ="CustomResourceDefinition"

    metadata {
        name = "certificates.certmanager.k8s.io"
        labels {
            app = "cert-manager"
            test = "la-la-connected-the-dots"
        }
    }

    yaml = <<EOF
  additionalPrinterColumns:
  - JSONPath: .status.conditions[?(@.type=="Ready")].status
    name: Ready
    type: string
  - JSONPath: .spec.secretName
    name: Secret
    type: string
  - JSONPath: .spec.issuerRef.name
    name: Issuer
    type: string
    priority: 1
  - JSONPath: .status.conditions[?(@.type=="Ready")].message
    name: Status
    type: string
    priority: 1
  - JSONPath: .metadata.creationTimestamp
    description: |-
      CreationTimestamp is a timestamp representing the server time when this object was created. It is not guaranteed to be set in happens-before order across separate operations. Clients may not set this value. It is represented in RFC3339 form and is in UTC.

      Populated by the system. Read-only. Null for lists. More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#metadata
    name: Age
    type: date
  group: certmanager.k8s.io
  version: v1alpha1
  scope: Namespaced
  names:
    kind: Certificate
    plural: certificates
    shortNames:
    - cert
    - certs
EOF
}

@Tazer
Copy link

Tazer commented Feb 26, 2019

Cool @timthesinner,

Trying some more, I get quite an interesting output.

Cause just tried to rerun with the new version and got kubernetes_generic.cr-clusterissuer: customresourcedefinitions.apiextensions.k8s.io "clusterissuers.certmanager.k8s.io" already exists

Even without a successful run, So I tried kubectl delete crd clusterissuers.certmanager.k8s.io with response: customresourcedefinition.apiextensions.k8s.io "clusterissuers.certmanager.k8s.io" deleted

And did another run then I got the EOF error. and then I ran again and got kubernetes_generic.cr-clusterissuer: customresourcedefinitions.apiextensions.k8s.io "clusterissuers.certmanager.k8s.io" already exists

So seems the "create" is working but something with the response? Is there a good way I can debug it a bit better?

@timthesinner
Copy link
Author

Can you put your TF (redacted and focused on just your generic resources) in a gist for me?

You can run with TF_LOG=DEBUG terraform apply for more output. Will be honest that's a weak area in this PR, I have been going into the code and adding log statements to figure out what's going on.

@Tazer
Copy link

Tazer commented Feb 28, 2019

@timthesinner here you go https://gist.github.com/Tazer/1b0d8b1ff0c99b2e7bbb35e770061756

Seems also that I'm getting another error on one of the resources:

resource "kubernetes_generic" "cr-clusterissuer" {
  api_version = "apiextensions.k8s.io/v1beta1"
  kind        = "CustomResourceDefinition"

  metadata {
    name = "clusterissuers.certmanager.k8s.io"

    labels {
      app = "cert-manager"
    }
  }

  yaml = <<EOF
  group: certmanager.k8s.io
  version: v1alpha1
  names:
    kind: ClusterIssuer
    plural: clusterissuers
  scope: Cluster
EOF
}

Output:

* kubernetes_generic.cr-clusterissuer: 1 error(s) occurred:

* kubernetes_generic.cr-clusterissuer: strconv.Atoi: parsing "#": invalid syntax

Change

kubernetes_generic.cr-clusterissuer: Modifying... (ID: apiextensions.k8s.io/v1beta1/clusterissuers.certmanager.k8s.io)
  metadata.0.labels.%:                                           "0" => "1"
  metadata.0.labels.app:                                         "" => "cert-manager"
  spec.additionalPrinterColumns.#:                               "1" => ""
  spec.additionalPrinterColumns.0.JSONPath:                      ".metadata.creationTimestamp" => ""
  spec.additionalPrinterColumns.0.description:                   "CreationTimestamp is a timestamp representing the server time when this object was created. It is not guaranteed to be set in happens-before order across separate operations. Clients may not set this value. It is represented in RFC3339 form and is in UTC.\n\nPopulated by the system. Read-only. Null for lists. More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#metadata" => ""
  spec.additionalPrinterColumns.0.name:                          "Age" => ""
  spec.additionalPrinterColumns.0.type:                          "date" => ""
  spec.names.listKind:                                           "ClusterIssuerList" => ""
  spec.names.singular:                                           "clusterissuer" => ""
  spec.transitent-delete.additionalPrinterColumns.#:             "" => "'1' --> ''"
  spec.transitent-delete.additionalPrinterColumns.0.JSONPath:    "" => "'.metadata.creationTimestamp' --> ''"
  spec.transitent-delete.additionalPrinterColumns.0.description: "" => "'CreationTimestamp is a timestamp representing the server time when this object was created. It is not guaranteed to be set in happens-before order across separate operations. Clients may not set this value. It is represented in RFC3339 form and is in UTC.\n\nPopulated by the system. Read-only. Null for lists. More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#metadata' --> ''"
  spec.transitent-delete.additionalPrinterColumns.0.name:        "" => "'Age' --> ''"
  spec.transitent-delete.additionalPrinterColumns.0.type:        "" => "'date' --> ''"
  spec.transitent-delete.names.listKind:                         "" => "'ClusterIssuerList' --> ''"
  spec.transitent-delete.names.singular:                         "" => "'clusterissuer' --> ''"
  spec.transitent-delete.versions.#:                             "" => "'1' --> ''"
  spec.transitent-delete.versions.0.name:                        "" => "'v1alpha1' --> ''"
  spec.transitent-delete.versions.0.served:                      "" => "'true' --> ''"
  spec.transitent-delete.versions.0.storage:                     "" => "'true' --> ''"
  spec.versions.#:                                               "1" => ""
  spec.versions.0.name:                                          "v1alpha1" => ""
  spec.versions.0.served:                                        "true" => ""
  spec.versions.0.storage:                                       "true" => ""

So seems it tries to verify that resource

Guess it's this one spec.transitent-delete.additionalPrinterColumns.#: "" => "'1' --> ''" ?

@yacut
Copy link

yacut commented Mar 25, 2019

@timthesinner @Tazer any update on this?

@Tazer
Copy link

Tazer commented Mar 25, 2019

The current status is that,

Creating crds defination doesn't really work for me. ( Solved it by running kubectl via local exec for those) but works fine when creating crd resources.

@yacut
Copy link

yacut commented Apr 12, 2019

@timthesinner Are you still working on it? Do you need some help?

@barryib
Copy link

barryib commented Sep 3, 2019

Any update on this ? Any chance to see this merged ?

@JulienBreux
Copy link

Hi @timthesinner,

Do you need some help?

Tell me how can I help you?

Cheers

@cwebbtw
Copy link
Contributor

cwebbtw commented Feb 6, 2020

Is there an update on this? It would be great to have support for custom resource definitions, is there any support that can be provided on this PR to understand what work is left to do.

@hashicorp-cla
Copy link

CLA assistant check

Thank you for your submission! We require that all contributors sign our Contributor License Agreement ("CLA") before we can accept the contribution. Read and sign the agreement

Learn more about why HashiCorp requires a CLA and what the CLA includes

Have you signed the CLA already but the status is still pending? Recheck it.

@aareet aareet added the hold label Jan 25, 2021
@alexsomesan
Copy link
Member

Quick update here: the recently released v0.3.1 of the Kubernetes Alpha provider has quite usable support for CRDs. I would encourage everyone to give that a try a provide us with feedback. That would help us to graduate that provider to GA as soon as possible.

Base automatically changed from master to main March 23, 2021 15:53
@jrhouston
Copy link
Collaborator

Closing this as kubernetes_manifest is now in the provider.

@jrhouston jrhouston closed this Sep 23, 2021
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.