Updates for gmsa webhook for k8s version 1.22

[jsturtevant]

in #29 the certificates.k8s.io/v1 was missing the signerName

This includes all changes from #29 and adds some additional needed updates. See the comment below: #31 (comment)

Fixes: kubernetes/kubernetes#102113 #32

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jsturtevant

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [jsturtevant]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[jsturtevant]

/cc @ionutbalutoiu

[k8s-ci-robot]

@jsturtevant: GitHub didn't allow me to request PR reviews from the following users: ionutbalutoiu.

Note that only kubernetes-sigs members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @ionutbalutoiu

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[ionutbalutoiu]

I'm not sure, but we might need to update spec.groups as well.

When using kubernetes.io/kubelet-serving as signerName, the docs say that:

Permitted subjects - organizations are exactly ["system:nodes"], common name starts with "system:node:".

It needs to be checked.

[jsturtevant]

@sponte @ionutbalutoiu would you be able try this out? I have had success with deploying the v1 versions with these changes.

[ionutbalutoiu]

@sponte @ionutbalutoiu would you be able try this out? I have had success with deploying the v1 versions with these changes.

Hello James,

I tried these changes, and indeed you can successfully deploy the v1 resources.

However, the admission webhook doesn't work properly anymore.

When I validate with my usual deployment using the GMSA credential spec, the deployment resource reports the following status condition:

  - lastTransitionTime: "2021-05-19T07:29:57Z"
    lastUpdateTime: "2021-05-19T07:29:57Z"
    message: 'Internal error occurred: failed calling webhook "admission-webhook.windows-gmsa.sigs.k8s.io":
      expected webhook response of admission.k8s.io/v1, Kind=AdmissionReview, got
      /, Kind='
    reason: FailedCreate
    status: "True"
    type: ReplicaFailure

and the deployment replica pods never spawn.

So, I reverted ValidatingWebhookConfiguration and MutatingWebhookConfiguration to v1beta1, and I don't get that anymore.

But, then during the validation, the pods spawn, and they crash with RunContainerError reporting:

failed to start container "fac722f8b45378cf2fde4986234c6c41c26dd265afa04b422419c5eed45f30ff": Error response from daemon: hcsshim::CreateComputeSystem fac722f8b45378cf2fde4986234c6c41c26dd265afa04b422419c5eed45f30ff: winapi error #3489660941

So, I reverted the GMSA CRD to v1beta1, and everything works as expected.

In conclusion, only certificates.k8s.io/v1 changes work for me.

[sponte]

Hello @jsturtevant , thanks for following up. I can confirm that the version from `` commit is working. Output below:

done.
GMSA Webhooks setup
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0 154 100 154 0 0 1013 0 --:--:-- --:--:-- --:--:-- 1013
9 40.9M 9 4096k 0 0 10.6M 0 0:00:03 --:--:-- 0:00:03 10.6M 51 40.9M 51 21.2M 0 0 14.5M 0 0:00:02 0:00:01 0:00:01 15.9M 40.9M 100 40.9M 0 0 20.2M 0 0:00:02 0:00:02 --:--:-- 22.3M
Property "clusters.local.certificate-authority-data" set.
Context "local" created.
Generating RSA private key, 2048 bit long modulus (2 primes)
...............................................................................................................+++++
............+++++
e is 65537 (0x010001)
certificatesigningrequest.certificates.k8s.io/gmsa-webhook.gmsa-webhook created
NAME AGE SIGNERNAME REQUESTOR CONDITION
gmsa-webhook.gmsa-webhook 0s kubernetes.io/kubelet-serving system:serviceaccount:gmsa:gmsa-setup Pending
certificatesigningrequest.certificates.k8s.io/gmsa-webhook.gmsa-webhook approved
customresourcedefinition.apiextensions.k8s.io "gmsacredentialspecs.windows.k8s.io" deleted
customresourcedefinition.apiextensions.k8s.io/gmsacredentialspecs.windows.k8s.io created
namespace/gmsa-webhook unchanged
secret/gmsa-webhook configured
serviceaccount/gmsa-webhook unchanged
clusterrole.rbac.authorization.k8s.io/gmsa-webhook-gmsa-webhook-rbac-role unchanged
clusterrolebinding.rbac.authorization.k8s.io/gmsa-webhook-gmsa-webhook-binding-to-gmsa-webhook-gmsa-webhook-rbac-role unchanged
deployment.apps/gmsa-webhook unchanged
service/gmsa-webhook unchanged
validatingwebhookconfiguration.admissionregistration.k8s.io/gmsa-webhook configured
mutatingwebhookconfiguration.admissionregistration.k8s.io/gmsa-webhook configured

*** Windows GMSA Admission Webhook successfully deployed! ***
*** You can remove it by running /usr/local/bin/kubectl delete -f webhook-manifests.yml ***

However, admission of a pod is failing with the following message:

After reverting to previous configuration, things seem to work.

BTW: The setup script is not idempotent, every time it runs, it wipes out my GMSACredentialSpec objects. It would be good to be able to re-apply the script within existing environment, without destroying existing objects

[jsturtevant]

Thanks for validation, I will look into why the ValidatingWebhookConfiguration and MutatingWebhookConfiguration to v1beta1 changes are failing.

What are the versions of the Kubernetes you are both working from? The bump to the v1 versions is required for 1.22.

[sponte]

Hello, I'm on AKS 1.19.7 at the moment.

[ionutbalutoiu]

What are the versions of the Kubernetes you are both working from? The bump to the v1 versions is required for 1.22.

Hello! I'm using AKS and it's deployed with v1.20.5 (it was the newest version available when I deployed it).

If we are deploying versions < v1.22, we still have to use v1beta1 ?

[sponte]

How about creating tagged releases corresponding to k8s versions? ​

…

On May 19 2021, at 7:44 pm, Ionut BALUTOIU ***@***.***> wrote: > > What are the versions of the Kubernetes you are both working from? The bump to the v1 versions is required for 1.22. > Hello! I'm using AKS and it's deployed with v1.20.5 (it was the newest version available when I deployed it). If we are deploying versions < v1.22, we still have to use v1beta1 ? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub (#31 (comment)), or unsubscribe (https://github.com/notifications/unsubscribe-auth/AAALS3DPFAHZKZWI6R5JYGLTOQBKHANCNFSM45BBI7NQ).

[jsturtevant]

If we are deploying versions < v1.22, we still have to use v1beta1 ?

according to https://kubernetes.io/docs/reference/using-api/deprecation-guide/#webhook-resources-v122 v1 should be available from 1.16+. I think the code in the webhook might need to be updated to handle the new versions.

How about creating tagged releases corresponding to k8s versions?

Doesn't look like this project has an tag/releases. I wasn't a maintainer but version this repo is a good idea (maybe not to k8s releases but in general). I think at this point the project is pretty stable so we can tag the version before these changes and then create a new one.

[ionutbalutoiu]

If we are deploying versions < v1.22, we still have to use v1beta1 ?

according to https://kubernetes.io/docs/reference/using-api/deprecation-guide/#webhook-resources-v122 v1 should be available from 1.16+. I think the code in the webhook might need to be updated to handle the new versions.

How about creating tagged releases corresponding to k8s versions?

Doesn't look like this project has an tag/releases. I wasn't a maintainer but version this repo is a good idea (maybe not to k8s releases but in general). I think at this point the project is pretty stable so we can tag the version before these changes and then create a new one.

We could have a release tag with the codebase before these v1 changes. However, I don't think it's worth since it will be based on a deprecated API usage. That's only valuable for people deploying K8s less than v1.16.

To be honest, I don't think there are many production environments (if any!) with Windows + gMSA having Kubernetes release below v1.16.

I believe that we should have the webhook working only with the stable API version. And that should be maintained going forward. Once that's stable enough, we should have a release tag.

[zenodhaene]

But, then during the validation, the pods spawn, and they crash with RunContainerError reporting:
failed to start container "fac722f8b45378cf2fde4986234c6c41c26dd265afa04b422419c5eed45f30ff": Error response from daemon: hcsshim::CreateComputeSystem fac722f8b45378cf2fde4986234c6c41c26dd265afa04b422419c5eed45f30ff: winapi error #3489660941
So, I reverted the GMSA CRD to v1beta1, and everything works as expected.
In conclusion, only certificates.k8s.io/v1 changes work for me.

This problem is caused by v1 no longer accepting unstructured objects in the GMSA spec. Following modifications made the winapi error disappear. You can validate the problem by inspecting any created gmsacredentialspec and seeing that the credspec property is an empty object. (I'm on kubernetes v1.21.1)

apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
  name: gmsacredentialspecs.windows.k8s.io
  annotations:
    "api-approved.kubernetes.io": "https://github.com/kubernetes/enhancements/tree/master/keps/sig-windows/689-windows-gmsa"
spec:
  group: windows.k8s.io
  versions:
  - name: v1alpha1
    served: true
    storage: true
    schema:
      openAPIV3Schema:
        type: object
        properties:
          credspec:
            description: GMSA Credential Spec
            type: object
            properties:
              ActiveDirectoryConfig:
                type: object
                properties:
                  GroupManagedServiceAccounts:
                    type: array
                    items:
                      type: object
                      properties:
                        Name:
                          type: string
                        Scope:
                          type: string
              CmsPlugins:
                type: array
                items:
                  type: string
              DomainJoinConfig:
                type: object
                properties:
                  DnsName:
                    type: string
                  DnsTreeName:
                    type: string
                  Guid:
                    type: string
                  MachineAccountName:
                    type: string
                  NetBiosName:
                    type: string
                  Sid:
                    type: string
                  
  names:
    kind: GMSACredentialSpec
    plural: gmsacredentialspecs
  scope: Cluster

Perhaps this can be included in this PR?

[ionutbalutoiu]

But, then during the validation, the pods spawn, and they crash with RunContainerError reporting:
failed to start container "fac722f8b45378cf2fde4986234c6c41c26dd265afa04b422419c5eed45f30ff": Error response from daemon: hcsshim::CreateComputeSystem fac722f8b45378cf2fde4986234c6c41c26dd265afa04b422419c5eed45f30ff: winapi error #3489660941
So, I reverted the GMSA CRD to v1beta1, and everything works as expected.
In conclusion, only certificates.k8s.io/v1 changes work for me.

This problem is caused by v1 no longer accepting unstructured objects in the GMSA spec. Following modifications made the winapi error disappear. You can validate the problem by inspecting any created gmsacredentialspec and seeing that the credspec property is an empty object. (I'm on kubernetes v1.21.1)

apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
  name: gmsacredentialspecs.windows.k8s.io
  annotations:
    "api-approved.kubernetes.io": "https://github.com/kubernetes/enhancements/tree/master/keps/sig-windows/689-windows-gmsa"
spec:
  group: windows.k8s.io
  versions:
  - name: v1alpha1
    served: true
    storage: true
    schema:
      openAPIV3Schema:
        type: object
        properties:
          credspec:
            description: GMSA Credential Spec
            type: object
            properties:
              ActiveDirectoryConfig:
                type: object
                properties:
                  GroupManagedServiceAccounts:
                    type: array
                    items:
                      type: object
                      properties:
                        Name:
                          type: string
                        Scope:
                          type: string
              CmsPlugins:
                type: array
                items:
                  type: string
              DomainJoinConfig:
                type: object
                properties:
                  DnsName:
                    type: string
                  DnsTreeName:
                    type: string
                  Guid:
                    type: string
                  MachineAccountName:
                    type: string
                  NetBiosName:
                    type: string
                  Sid:
                    type: string
                  
  names:
    kind: GMSACredentialSpec
    plural: gmsacredentialspecs
  scope: Cluster

Perhaps this can be included in this PR?

I can confirm that this change fixes the winapi error. +1 to be included in this PR.

The only remaining item is making the AdmissionWebhook container image to be compatible with v1.

[jsturtevant]

I've pulled in changes from #29 and we now have CI (#35 and #36) on the pull requests and I am see the same errors reported above 👍

     utils.go:15:  Error from server (InternalError): error when creating "tmp/cannot-update-gmsa-pod-level-settings-631912892
/single-pod-with-gmsa.yml": Internal error occurred: failed calling webhook "admission-webhook.windows-gmsa.sigs.k8s.io": 
expected webhook response of admission.k8s.io/v1, Kind=AdmissionReview, got /, Kind=

[jsturtevant]

expected webhook response of admission.k8s.io/v1, Kind=AdmissionReview, got /, Kind=

Required updates to the response to use the latest version. Docs here: https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/

[jsturtevant]

Tests pass 🚀

Before merging need to build an push a valid image to a container registry. The previous image was on wk88/k8s-gmsa-webhook docker hub which is not community owned. Working on that now.

[jsturtevant]

/hold

[jsturtevant]

/unhold

@zenodhaene @ionutbalutoiu @sponte this is ready for review. If you could test it out that would be added validation 👍

I've moved the image over from the personal repo to the github packages: https://github.com/kubernetes-sigs/windows-gmsa/packages/825309 This will automatically be used be the deploy script.

docker pull docker.pkg.github.com/kubernetes-sigs/windows-gmsa/k8s-gmsa-webhook:latest

[sponte]

Hello, tried to implement it but docker image is not public i.e. it requires PAT token for reading packages. How can you feed ImagePullSecret into webhook deployment?

Alternatively, you can migrate to github container registry to enable anonymous access.

[ionutbalutoiu]

Hello, tried to implement it but docker image is not public i.e. it requires PAT token for reading packages. How can you feed ImagePullSecret into webhook deployment?

Alternatively, you can migrate to github container registry to enable anonymous access.

I can confirm this. Being an image for public consumption, I believe we should have anonymous access.

In the meantime, to confirm that all the changes are working, I re-built the Docker image with the changes from this PR, and temporarily published it to:

docker.io/ionutbalutoiu/k8s-gmsa-webhook:latest

Everything worked for me. Thank-you!

[sponte]

Using @ionutbalutoiu's image also worked for me too. The docker.pkg.github.com/kubernetes-sigs/windows-gmsa/k8s-gmsa-webhook:latest one had an issue with the layer, but haven't really investigated it properly

[jsturtevant]

Hello, tried to implement it but docker image is not public i.e. it requires PAT token for reading packages. How can you feed ImagePullSecret into webhook deployment?

ah, sorry I thought I tested it but must have had cached credentials.

Alternatively, you can migrate to github container registry to enable anonymous access.

This is what I looked at first but its not enabled for the kubenetes-sig org as of right now.

We have a sigwindowstools dockerhub repo which the sig-windows leads have access too, I'll move it there for now.

Thanks for testing it out!

[jsturtevant]

@ionutbalutoiu @sponte would you try this image?

docker pull sigwindowstools/k8s-gmsa-webhook:latest

[sponte]

This is now working for me :) Thanks

[ionutbalutoiu]

The new image works for me too. Thank-you!