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

"pulumi preview" needs 'patch' permission when SSA is enabled #2213

Closed
antifuchs opened this issue Oct 25, 2022 · 11 comments · Fixed by pulumi/pulumi-hugo#2237
Closed

"pulumi preview" needs 'patch' permission when SSA is enabled #2213

antifuchs opened this issue Oct 25, 2022 · 11 comments · Fixed by pulumi/pulumi-hugo#2237
Labels
area/server-side-apply customer/feedback Feedback from customers kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed
Milestone

Comments

@antifuchs
Copy link

What happened?

Thanks to pulumi/pulumi-eks#800 we got a preview of what enabling SSA via pulumi-k8s 3.22.0 does to our infra-as-code config (we pin 3.21.4):

  • We run pulumi preview with kubernetes credentials that only allow read, list, watch of all resources (never create/modify).
  • The pulumi-eks provider creates a pulumi-kubernetes provider at version 3.22.0, with default values - enabling SSA
  • When SSA is enabled, it attempts to modify the aws-auth configmap in the cluster.

This results in the following error message:

    kubernetes:core/v1:ConfigMap mzcloud-staging-system-nodeAccess  error: configmaps "aws-auth" is forbidden: User "gha-readonly-oidc:GithubActionsSession-pulumi-preview" cannot patch resource "configmaps" in API group "" in the namespace "kube-system"

Steps to reproduce

  1. create a cluster role that's similar to the following code:
          ro_role = k8s.rbac.v1.ClusterRole(
              f"{name}-gha-readonly-access",
              metadata=k8s.meta.v1.ObjectMetaArgs(
                  name="gha-readonly-access",
              ),
              rules=[
                  k8s.rbac.v1.PolicyRuleArgs(
                      api_groups=["*"],
                      resources=["*"],
                      verbs=["get", "list", "watch"],
                  )
              ],
              opts=pulumi.ResourceOptions(
                  provider=self.provider,
                  parent=self,
              ),
          )
          ro_group_name = "gha-readonly-access"
          k8s.rbac.v1.ClusterRoleBinding(
              f"{name}-gha-readonly-access",
              metadata=k8s.meta.v1.ObjectMetaArgs(name="gha-readonly-access"),
              subjects=[k8s.rbac.v1.SubjectArgs(kind="Group", name=ro_group_name)],
              role_ref=k8s.rbac.v1.RoleRefArgs(
                  kind="ClusterRole",
                  name=ro_role.metadata.name,
                  api_group="rbac.authorization.k8s.io",
              ),
              opts=pulumi.ResourceOptions(
                  provider=self.provider,
                  parent=self,
              ),
          )
          role_mappings_list.append(
              {
                  "rolearn": stack_config.GHA_READONLY_ROLE,
                  "username": r"gha-readonly-oidc:{{SessionName}}",
                  "groups": [ro_group_name],
              },
          )
  1. Configure the kubernetes credentials to use that role to authenticate into the EKS cluster
  2. Use pulumi-eks to define an EKS cluster with a given role/authorization mapping
  3. Run pulumi preview with those read-only credentials
  4. Observe the error above.

Expected Behavior

pulumi preview should never attempt to modify any data.

Actual Behavior

pulumi preview attempts to modify data and is (thankfully!) not permitted to.

Output of pulumi about

CLI
Version      3.42.0
Go Version   go1.19.1
Go Compiler  gc

Plugins
NAME              VERSION
aws               5.16.0
cloudinit         1.3.0
command           0.5.2
crds              0.0.0
docker-buildkit   0.1.20
eks               0.40.0
fivetran          0.1.6
frontegg          0.2.24
honeycomb         0.0.13
kubernetes        3.21.4
kubernetes-proxy  0.1.3
linkerd-link      0.0.7
postgresql        3.6.0
postgresql-exec   0.1.1
python            unknown
random            4.8.2
tls               4.6.1

Host
OS       nixos
Version  22.05 (Quokka)
Arch     x86_64

This project is written in python: executable='/home/asf/Hacks/cloud/venv/bin/python3' version='3.9.13

(redacted stack and list of resources)

Additional context

We're attempting to "pin" the pulumi-kubernetes provider at 3.21.4 while we figure out how to live without SSA for the time being - ineffectively. I'm going to post about our problems in pulumi/pulumi#10758 to hopefully get us a solution for opting into our own upgrade cadence.

Contributing

Vote on this issue by adding a 👍 reaction.
To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

@antifuchs antifuchs added kind/bug Some behavior is incorrect or out of spec needs-triage Needs attention from the triage team labels Oct 25, 2022
@ringods ringods added the p1 A bug severe enough to be the next item assigned to an engineer label Oct 25, 2022
@lblackstone
Copy link
Member

lblackstone commented Oct 25, 2022

Hi @antifuchs, thanks for the report. I want to clarify that the preview step does not modify any resources. The Server-Side Apply feature uses the Patch API, but it's using the dry-run mode. The permissions you listed, ["get", "list", "watch"], work for a client-side preview, but are not sufficient to run a server-side preview.

We're attempting to "pin" the pulumi-kubernetes provider at 3.21.4 while we figure out how to live without SSA for the time being - ineffectively.

To disable the Server-Side Apply feature, you have the following options to set Provider configuration:

  1. Set the enableServerSideApply parameter to false on your Provider resource.
  2. Set the environment variable PULUMI_K8S_ENABLE_SERVER_SIDE_APPLY="false"
  3. Set the stack config pulumi config set kubernetes:enableServerSideApply false

@lblackstone lblackstone added customer/feedback Feedback from customers and removed needs-triage Needs attention from the triage team labels Oct 25, 2022
@antifuchs
Copy link
Author

Thanks for the clarification! I'd argue that the dry-run distinction is pretty meaningless if there exists no set of k8s permissions exists that allows preview but doesn't allow untrusted code to create resources via a pull request (:

Anyway, I have tried the workarounds that you list:

  1. Number one doesn't work (setting the parameter on the provider resource) because pulumi-eks creates one with defaults; that was our status quo.
  2. Setting the env variable does work. The k8s provider created by pulumi-eks still seems to create a provider with enabled SSA.
  3. Setting the pulumi stack config value alone does not work: Same behavior as with scenario one, SSA not disabled globally.

I'm not sure why scenario two doesn't help - here's our diff from each our stack config files (forgive the imprecise comment please, it's verbatim from us trying this change before you posted):

diff --git a/Pulumi.personal.yaml b/Pulumi.personal.yaml
index 45a5ed54..81f49816 100644
--- a/Pulumi.personal.yaml
+++ b/Pulumi.personal.yaml
@@ -23,6 +23,7 @@ config:

   # Kubernetes provider configuration.
   kubernetes:kubeconfig: invalid
+  kubernetes:enableServerSideApply: false

   # ==> Materialize Cloud-specific configuration.
   mzcloud:admin_sso_role: arn:aws:iam::725780499770:role/AWSReservedSSO_Administrator_b08b525ab43925ee
diff --git a/Pulumi.production.yaml b/Pulumi.production.yaml
index b719b2ef..0b9c732b 100644
--- a/Pulumi.production.yaml
+++ b/Pulumi.production.yaml
@@ -39,6 +39,14 @@ config:
   # See: https://github.com/pulumi/pulumi/issues/3383
   kubernetes:kubeconfig: invalid

+  # Disable kubernetes server-side apply (SSA), which is buggy; Some
+  # providers like pulumi-eks pull in the "latest" version of the
+  # provider and create their own k8s providers with default values
+  # (https://github.com/pulumi/pulumi-eks/issues/800), which turns on
+  # SSA for their own operations, and that causes things like "pulumi
+  # preview" to attempt to modify resources on the server.
+  kubernetes:enableServerSideApply: false
+
   # ==> Materialize Cloud-specific configuration.
   # The role created for admins who sign in via SSO. We'll be able to fetch
   # this more dynamically when pulumi/pulumi-aws#1460 is resolved.
diff --git a/Pulumi.staging.yaml b/Pulumi.staging.yaml
index 661ab04c..6aad0f51 100644
--- a/Pulumi.staging.yaml
+++ b/Pulumi.staging.yaml
@@ -39,6 +39,14 @@ config:
   # See: https://github.com/pulumi/pulumi/issues/3383
   kubernetes:kubeconfig: invalid

+  # Disable kubernetes server-side apply (SSA), which is buggy; Some
+  # providers like pulumi-eks pull in the "latest" version of the
+  # provider and create their own k8s providers with default values
+  # (https://github.com/pulumi/pulumi-eks/issues/800), which turns on
+  # SSA for their own operations, and that causes things like "pulumi
+  # preview" to attempt to modify resources on the server.
+  kubernetes:enableServerSideApply: false
+
   # ==> Materialize Cloud-specific configuration.
   # The role created for admins who sign in via SSO. We'll be able to fetch
   # this more dynamically when pulumi/pulumi-aws#1460 is resolved.

@lblackstone
Copy link
Member

lblackstone commented Oct 25, 2022

I'd argue that the dry-run distinction is pretty meaningless if there exists no set of k8s permissions exists that allows preview but doesn't allow untrusted code to create resources via a pull request

I agree that this seems like an unfortunate side effect of the Kubernetes API design here. I'll have to look and see if there are any open issues upstream about that.

Edit: I found some related discussions upstream:
kubernetes/kubectl#981
kubernetes/kubernetes#95449

Setting the pulumi stack config value alone does not work: Same behavior as with scenario one, SSA not disabled globally.

I just double-checked the stack configuration locally and confirmed that the setting you indicated disables SSA mode for me. We'll need to do some more investigation to see if we can find out why that didn't work for you.

@antifuchs
Copy link
Author

I just double-checked the stack configuration locally and confirmed that the setting you indicated disables SSA mode for me. We'll need to do some more investigation to see if we can find out why that didn't work for you.

After discussion with @ringods, it sounds like this is due to the way pulumi-eks interacts with pulumi-kubernetes: It disregards the kubernetes:enableServerSideApply setting for the provider that it creates. The setting is correct for all the k8s providers that we create in our code directly, just not in the pulumi-eks created resources.

@ringods
Copy link
Member

ringods commented Oct 26, 2022

@antifuchs FYI pulumi/pulumi-eks#803

@squaremo
Copy link
Contributor

NB @pulumi/kubernetes v3.22.1 reverts to SSA being disabled by default. It sounds from the above like this will make the symptom go away -- pulumi-eks will use the most recent version of pulumi-kubernetes, with server-side apply disabled (again).

The problem of SSA needing elevated permissions for preview still remains (though largely out of our hands?), so I'll leave this issue open.

@lblackstone lblackstone added area/server-side-apply and removed p1 A bug severe enough to be the next item assigned to an engineer labels Oct 31, 2022
@squaremo
Copy link
Contributor

I'd argue that the dry-run distinction is pretty meaningless if there exists no set of k8s permissions exists that allows preview but doesn't allow untrusted code to create resources via a pull request

It is arguable, that is certain :-) --dry-run should mean "tell me what would happen, but don't actually do any of it", and for server side apply that includes things like validation, admission controllers and permissions. So, arguably, --dry-run should tell you if access is denied given your permissions, because that's something that would happen.

Assuming (or pretending) that there's an account with write permissions that you use for deployments: if there were some objects in your stack that that account couldn't update, you would want preview to tell you, wouldn't you?

@squaremo squaremo changed the title "pulumi preview" attempts to modify resources when SSA is enabled "pulumi preview" needs write permissions when SSA is enabled Nov 15, 2022
@squaremo
Copy link
Contributor

squaremo commented Nov 15, 2022

What we'd really need from the Kubernetes API is a verb something like "impersonate-for-dry-run", which lets you impersonate another account, but restricts your requests to only those marked as dry-run.

@lblackstone lblackstone changed the title "pulumi preview" needs write permissions when SSA is enabled "pulumi preview" needs 'patch' permission when SSA is enabled Nov 15, 2022
@lblackstone
Copy link
Member

I spent some more time reading through the upstream discussion about this, and it looks like patch permission is going to be a hard requirement of this feature. There were a few other users pushing for a way to filter previews by verb, but those discussions didn't go anywhere as far as I can tell.

Given that information, I see the following alternatives:

  1. Add/retain a client-side preview mode that doesn't use dry-run
  2. Use an admission controller on the cluster to restrict mutating operations

Unfortunately, both of these approaches have significant drawbacks. Fundamentally, the only way to get an accurate preview of changes with the current Kubernetes API is to allow patch permission. We will add documentation about this requirement to the provider, but I don't see a path to avoiding it at this time.

@michael-barker
Copy link

Any recommendations on how to safely do previews from a non-protected branch? Our current workflow uses the Pulumi GitLab integration to add comments to the MR with the results of the preview. When using server side apply, I believe this will fail unless that preview jobs have patch permissions which would not be secure to allow on a non-protected branch.

@lblackstone
Copy link
Member

Any recommendations on how to safely do previews from a non-protected branch? Our current workflow uses the Pulumi GitLab integration to add comments to the MR with the results of the preview. When using server side apply, I believe this will fail unless that preview jobs have patch permissions which would not be secure to allow on a non-protected branch.

Yeah, it's unfortunate that the upstream design here lumps the different types of operations together into the same verb. It seems like the least disruptive change for us may be to use some form of client-side preview as a fallback option. Unfortunately, client-side support is based on the last-applied-configuration annotation, which causes other problems.

Can you open a new issue with details on your requirements? I think the client-side fallback idea warrants further investigation, and would also be open to suggestions if you have ideas about this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/server-side-apply customer/feedback Feedback from customers kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants