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

kubernetes_manifest resource should NOT make any CRD call #1665

Open
bfanyuk opened this issue Mar 29, 2022 · 6 comments
Open

kubernetes_manifest resource should NOT make any CRD call #1665

bfanyuk opened this issue Mar 29, 2022 · 6 comments

Comments

@bfanyuk
Copy link

bfanyuk commented Mar 29, 2022

Description

If you try to plan/apply example from documentation then you will get an error unless you have list customresourcedefinitions permission.
Getting this another permission can be a whole big story.
In my case our devops team that is responsible to configure kubernetes cluster and I had to create a special ticket just for terraform.

But to apply example above no CRD call is needed in fact, just /v1/configmap call.
The same is relevant if I try to create something more complicated like

kind: Role
apiVersion: rbac.authorization.k8s.io/v1

I explicitly specified apiversion and kind and constructing the right api path should not be a big deal, in this case it is /v1/rbac.authorization.k8s.io.role

In the worst case maybe it makes sense to introduce another parameter that will define this path explicitly.
But this is anyway better than having to get another weird permission just to be able to execute terraform code.

Potential Terraform Configuration

The best option is to keep as is and derive api path from apiVersion and kind.

resource "kubernetes_manifest" "test-config-map" {
  manifest = {
    apiVersion = "v1"
    kind       = "ConfigMap"
    metadata   = {
      namespace = "default"
      name      = "test-config-map"
    }
    data = {
      "foo" = "bar"
    }
  }
}

Another option is to introduce a special parameter for api path:

resource "kubernetes_manifest" "test-config-map" {
  manifest = {
    apiVersion = "v1"
    kind       = "ConfigMap"
    apiPath    = "/v1/configmap" # this is a new parameter that points explicitly to the api path
    metadata   = {
      namespace = "default"
      name      = "test-config-map"
    }
    data = {
      "foo" = "bar"
    }
  }
}

References

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment
@alexsomesan
Copy link
Member

alexsomesan commented Mar 30, 2022

The provider needs to list CRDs, because it needs to determine if the resource type given in the manifest is defined by a CRD, the built-in API objects or an aggregated API server.

We cannot skip the CRD list step, because when a CRD is installed, its schema is also published in the cluster's OpenAPI document, so discovering the resource as part of that document does not distinguish between it being a CRD or a regular built in resource (they both apear in the OpenAPI document).

Finally, the provider needs to make this distinction because the schema in a CRD is defined using OpenAPIv3 syntax and the schema of built-in cluster resources is defined using OpenAPI v2 syntax. The API server will perform a lossy translation from OAPI v3 to v2 when publishing CRDs in the main OpenAPI document which is an imprecise aproximation of the schema defined by the CRD author. The provider cannot rely on this imprecise approximation as it may lead to corruption of the Terraform state when doing updates to such resources.

That being sad, we consider that allowing permissions to list the CRD objects in the cluster is not a security risk, since CRDs only define structural information and do no store any specific data that could be considered sensitive. We believe that this is a reasonable requirement so that we can offer a robust user experience around managing CRDs, which has been the primary goal in developing this provider.

We are however interested to hear about and evaluate any security concerns you or your ops teams might have about allowing API clients to list CRDs.

@bfanyuk
Copy link
Author

bfanyuk commented Apr 1, 2022

Hello, @alexsomesan !

Thanks a lot for such a detailed explanation. Maybe it worth placing to the documentation.

However let me clarify here a little bit.
I can understand that if I would use resource of CRD type I would need to list CRD.
But what if I have to create a non-CRD resource?
Let's have a look at these 2 configurations:

resource "kubernetes_config_map" "test-config-map" {
  metadata {
    namespace = "default"
    name      = "test-config-map"
  }
  data = {
    "foo" = "bar"
  }
}

resource "kubernetes_manifest" "test-config-map" {
  manifest = {
    apiVersion = "v1"
    kind       = "ConfigMap"
    metadata   = {
      namespace = "default"
      name      = "test-config-map"
    }
    data = {
      "foo" = "bar"
    }
  }
}

First one works without CRD call, second one requires CRD permission.
Don't they do the same operation in the end?
Maybe introducing a flag like "skipCRD" would help for these cases.

Answering your question about security risk - no, our ops don't consider this as security risk by itself.
However, they spent some days to research this problem and to configure this correctly.
It would be great if this additional work could be avoided.

@acainelli
Copy link

@bfanyuk I know it's been a long time. Did you find any workaround for this? I came to the same issue a few months ago. Unfortunately for me it's not possible at all to use kubernetes_manifest as i use VMWare Tanzu to deploy k8s cluster as kubernetes resources, and by default tanzu create users without CRD list permission. So my workaround was to parse my manifest in one resource and i use local-exec provisioner to run ansible and apply CRDs. It's dirty but it works :). However, finding a way to use kubernetes_manifest would be nice, but i am limited on Tanzu side.

@bfanyuk
Copy link
Author

bfanyuk commented Feb 21, 2023

Hello, @acainelli!
My workaround was to ask our devops team to add this permission.
Feel free to vote for this issue to increase it's priority.

@slauger
Copy link

slauger commented Feb 21, 2023

It would be great if this behavior could be changed. The issue can also occur in OpenShift environments. A normal user does not have permission to fetch the CRDs (and maybe the cluster admin don't want to give you this permission).

In such scenarios the resource is currently useless.

@luisdavim
Copy link

The provider needs to list CRDs, because it needs to determine if the resource type given in the manifest is defined by a CRD, the built-in API objects or an aggregated API server.

We cannot skip the CRD list step, because when a CRD is installed, its schema is also published in the cluster's OpenAPI document, so discovering the resource as part of that document does not distinguish between it being a CRD or a regular built in resource (they both apear in the OpenAPI document).

Finally, the provider needs to make this distinction because the schema in a CRD is defined using OpenAPIv3 syntax and the schema of built-in cluster resources is defined using OpenAPI v2 syntax. The API server will perform a lossy translation from OAPI v3 to v2 when publishing CRDs in the main OpenAPI document which is an imprecise aproximation of the schema defined by the CRD author. The provider cannot rely on this imprecise approximation as it may lead to corruption of the Terraform state when doing updates to such resources.

That being sad, we consider that allowing permissions to list the CRD objects in the cluster is not a security risk, since CRDs only define structural information and do no store any specific data that could be considered sensitive. We believe that this is a reasonable requirement so that we can offer a robust user experience around managing CRDs, which has been the primary goal in developing this provider.

We are however interested to hear about and evaluate any security concerns you or your ops teams might have about allowing API clients to list CRDs.

This is not quite true, you can use kubectl to apply a manifest representing a custom resource even if you don't have permissions to list CRDs...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants