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

Add doc on how to setup localhost serving Admission Webhook #8

Closed
wants to merge 1 commit into from

Conversation

tkashem
Copy link
Contributor

@tkashem tkashem commented Dec 3, 2019

No description provided.

@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Dec 3, 2019
- name: clusterresourceoverrides.admission.autoscaling.openshift.io
clientConfig:
# serving on localhost.
url: https://localhost:9443/apis/admission.autoscaling.openshift.io/v1/clusterresourceoverrides
Copy link
Contributor Author

@tkashem tkashem Dec 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

serving on localhost of each node where kube API server is also hosted.

node-role.kubernetes.io/master: ''

# enable hostNetwork to do localhost serving
hostNetwork: true
Copy link
Contributor Author

@tkashem tkashem Dec 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want to serve on localhost of the host network of each master node.

args:
# the server binds to 127.0.0.1 to disable external connection.
# pod readiness and liveness check does not work.
- "--bind-address=127.0.0.1"
Copy link
Contributor Author

@tkashem tkashem Dec 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bind to 127.0.0.1 to allow connection from localhost exclusively.

Forbidden: "/apis/admission.autoscaling.openshift.io/v1/clusterresourceoverrides?timeout=30s", Reason: ""
```

To solve this issue grant `create` verb on the designated API resource to `system:anonymous`.
Copy link
Contributor Author

@tkashem tkashem Dec 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am I doing something wrong here or is there a better way to solve thi?

@tkashem
Copy link
Contributor Author

tkashem commented Dec 3, 2019

/assign @deads2k

@@ -0,0 +1,178 @@
## Admission Webhooks over localhost
* Use `DaemonSet`: The DaemonSet controller can make Pods even when the scheduler has not been started, which can help cluster bootstrap.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not true, you still need a scheduler. But you don't need the SDN to be working.

## Admission Webhooks over localhost
* Use `DaemonSet`: The DaemonSet controller can make Pods even when the scheduler has not been started, which can help cluster bootstrap.
* Use `hostnetwork` for `PodSpec`
* The pods must be scheduled on to each master node so that core API server can access the webhook.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kube-apiserver


Note that we are not using the following:
* The `DaemonSet` pods are not fronted by any `Service`
* No API aggregation, the Admission webhook can not be reached via the `kubernetes.default.svc` service. So we don't get the advantages to registering the webhook server as an aggregated API.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As an exercise for our future, can you see if it's possible to register as a service. Imagine the ImagePolicy admission plugin which we want for kube-apiserver and openshift-apiserver. Doesn't have to be now and we don't want it for this plugin.

### Setup
* Grant the `ServiceAccount` of the `DaemonSet` access to the `hostnetwork` `SCC`
```bash
oc adm policy add-scc-to-user hostnetwork system:serviceaccount:cluster-resource-override:clusterresourceoverride
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of this, use the oc create rolebinding technique

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

```bash
oc adm policy add-scc-to-user hostnetwork system:serviceaccount:cluster-resource-override:clusterresourceoverride
```
* Grant `create` verb on the designated API resource of the API group the admission webhook exposes to `system:anonymous`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

specify which resource

- name: clusterresourceoverrides.admission.autoscaling.openshift.io
clientConfig:
# serving on localhost.
url: https://localhost:9443/apis/admission.autoscaling.openshift.io/v1/clusterresourceoverrides
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like a non-443, so we can start numbering these. How about 9400?

@tkashem
Copy link
Contributor Author

tkashem commented Dec 4, 2019

The feedback has been addressed. A new PR has been opened in enhancements -openshift/enhancements#138

@deads2k deads2k added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 6, 2019
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by: tkashem

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:

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

@tkashem
Copy link
Contributor Author

tkashem commented Dec 8, 2019

/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 8, 2019
@openshift-bot
Copy link
Contributor

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci-robot openshift-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 23, 2020
@openshift-bot
Copy link
Contributor

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

@openshift-ci-robot openshift-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Oct 24, 2020
@openshift-bot
Copy link
Contributor

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

@openshift-ci-robot
Copy link
Contributor

@openshift-bot: Closed this PR.

In response to this:

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants