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

Webhook ServiceReference keeps calling 443 port when there is only one non-443 port in ServiceSpec #61510

Closed
freehan opened this issue Mar 22, 2018 · 5 comments · Fixed by #62496
Assignees
Labels
area/admission-control kind/bug Categorizes issue or PR as related to a bug. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery.

Comments

@freehan
Copy link
Contributor

freehan commented Mar 22, 2018

What happened:

Deployed a webhook backend and service as follows:
Please note that there is only 8443 port on the service.

apiVersion: extensions/v1beta1
kind: Deployment
metadata:
  labels:
    run: webhook
  name: webhook
  namespace: kube-system
spec:
  template:
    metadata:
      labels:
        run: webhook
    spec:
      containers:
      - args:
        - --tls-cert-file=/webhook.local.config/certificates/tls.crt
        - --tls-private-key-file=/webhook.local.config/certificates/tls.key
        - --alsologtostderr
        - -v=4
        - 2>&1
        image: xxxxxxxxxxxxxxxxxxxxxxxxxxx
        imagePullPolicy: Always
        name: webhook
        volumeMounts:
        - mountPath: /webhook.local.config/certificates
          name: webhook-certs
          readOnly: true
      volumes:
      - name: webhook-certs
        secret:
          defaultMode: 420
          secretName: webhook-secret
---
apiVersion: v1
kind: Service
metadata:
  labels:
    run: webhook
  name: webhook-service
  namespace: kube-system
spec:
  ports:
    - port: 8443
      protocol: TCP
      targetPort: 8443
  selector:
    run: "webhook"

Register the webhook as follows:

{  
   "metadata":{  
      "name":"webhook-config",
      "selfLink":"/apis/admissionregistration.k8s.io/v1beta1/mutatingwebhookconfigurations/webhook-config",
      "uid":"d0ea605f-2d68-11e8-a2a7-42010a280002",
      "resourceVersion":"867428",
      "generation":1,
      "creationTimestamp":"2018-03-22T00:34:43Z"
   },
   "webhooks":[  
      {  
         "name":"adding-annotation.k8s.io",
         "clientConfig":{  
            "service":{  
               "namespace":"kube-system",
               "name":"webhook-service",
               "path":"/mutating-pods"
            },
            "caBundle":"xxxxxxxxxxxxxxxxxxxx"
         },
         "rules":[  
            {  
               "operations":[  
                  "CREATE"
               ],
               "apiGroups":[  
                  ""
               ],
               "apiVersions":[  
                  "v1"
               ],
               "resources":[  
                  "pods"
               ]
            }
         ],
         "failurePolicy":"Fail",
         "namespaceSelector":{  

         }
      }
   ]
}

Got the following error when creating pods:

Error creating: Internal error occurred: failed calling admission webhook "adding-annotation.k8s.io": Post https://webhook-service.kube-system.svc:443/mutating-pods?timeout=30s: no service port "443" found for service "webhook-service"

What you expected to happen:
Based on https://github.com/kubernetes/api/blob/master/admissionregistration/v1beta1/types.go#L257, https://webhook-service.kube-system.svc:8443/mutating-pods should be called. Not the 443 port shown in the error message.

How to reproduce it (as minimally and precisely as possible):
Just deploy the same setup and register webhook.

Anything else we need to know?:

Environment:

  • Kubernetes version (use kubectl version): HEAD
    Client Version: version.Info{Major:"1", Minor:"10+", GitVersion:"v1.10.0-alpha.1.97+2bbae9ab581bf6-dirty", GitCommit:"2bbae9ab581bf6fc0fb491136c42673c745c190d", GitTreeState:"dirty", BuildDate:"2018-01-22T20:00:24Z", GoVersion:"go1.9.1", Compiler:"gc", Platform:"linux/amd64"}
    Server Version: version.Info{Major:"1", Minor:"11+", GitVersion:"v1.11.0-alpha.0.470+890bd2174cf566", GitCommit:"890bd2174cf566e663c9cc0768799f7a94dbf6f0", GitTreeState:"clean", BuildDate:"2018-03-12T19:36:42Z", GoVersion:"go1.9.3", Compiler:"gc", Platform:"linux/amd64"}
  • Cloud provider or hardware configuration: GCE
  • OS (e.g. from /etc/os-release): COS
  • Kernel (e.g. uname -a): COS
@freehan freehan added kind/bug Categorizes issue or PR as related to a bug. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. labels Mar 22, 2018
@caesarxuchao
Copy link
Member

@freehan, thanks for the detailed bug report!

@liggitt
Copy link
Member

liggitt commented Mar 22, 2018

I actually think that the API specification (or documentation) must change. Not all API servers will have access to look up the Service object and see what ports it defines. If we want to support ports other than 443, the port information must be specified in the webhook object, not discovered by direct Service object lookup

@lavalamp
Copy link
Member

/assign @jennybuckley

@jennybuckley
Copy link

jennybuckley commented Apr 12, 2018

@liggitt
Looks like it will always use 443 no matter what ports are available, right? Should our documentation reflect this or should it behave differently? https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/config/client.go#L133

(also just sets 443 in the custom dial wrapper thing https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/config/serviceresolver.go#L44)

@liggitt
Copy link
Member

liggitt commented Apr 12, 2018

As a first step, I would adjust the documentation. As noted in #61510 (comment), if we want to support ports other than 443, that information needs to be added to the webhook config object

k8s-github-robot pushed a commit that referenced this issue Apr 13, 2018
Automatic merge from submit-queue (batch tested with PRs 61306, 60270, 62496, 62181, 62234). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Update WebhookClientConfig documentation regarding service ports

**What this PR does / why we need it**:
Dynamic admission webhooks backed by services [will always use 443](https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/config/client.go#L133) no matter what ports are available. Our [current documentation](https://github.com/kubernetes/api/blob/master/admissionregistration/v1beta1/types.go#L257-L259) says that "If there is only one port open for the service, that port will be used."

This PR fixes that piece of documentation.
In the future we may wish to support specifying ports other than 443, but the documentation should be fixed first.

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes #61510

**Release note**:
```release-note
NONE
```

/sig api-machinery
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/admission-control kind/bug Categorizes issue or PR as related to a bug. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants