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

Move cert-manager execution to a new pod #716

Merged
merged 1 commit into from
Mar 25, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 68 additions & 31 deletions deploy/handler/operator.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -56,38 +56,85 @@ spec:
value: "False"
- name: PROFILER_PORT
value: "6060"
- name: CA_ROTATE_INTERVAL
value: {{ .CARotateInterval | default "8760h0m0s" }}
- name: CA_OVERLAP_INTERVAL
value: {{ .CAOverlapInterval | default "24h0m0s" }}
- name: CERT_ROTATE_INTERVAL
value: {{ .CertRotateInterval | default "4380h0m0s" }}
- name: CERT_OVERLAP_INTERVAL
value: {{ .CertOverlapInterval | default "24h0m0s" }}
ports:
- containerPort: 8443
- containerPort: 9443
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are we changing this?

Copy link
Member Author

Choose a reason for hiding this comment

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

We are going to use directly the webhook server from controller-runtime so we just use the defaults there since there are good enough for us and we don't have to add more code to change it to 8443, same happends to TLS cert directory.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ack, Thanks

Copy link
Collaborator

Choose a reason for hiding this comment

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

will it have future value to put this in a env var (prevent future collisions)?

Copy link
Member Author

Choose a reason for hiding this comment

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

we don't run it with network=host so is not going to collision, I doubt we are going to have multiple containers at the same pod with the same port.

name: webhook-server
protocol: TCP
readinessProbe:
httpGet:
path: /readyz
port: webhook-server
scheme: HTTPS
httpHeaders:
- name: Content-Type
value: application/json
initialDelaySeconds: 10
periodSeconds: 10
volumeMounts:
- name: tls-key-pair
readOnly: true
mountPath: /etc/webhook/certs
mountPath: /tmp/k8s-webhook-server/serving-certs/
Copy link
Collaborator

Choose a reason for hiding this comment

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

genereally speaking, why in /tmp/...?

Copy link
Member Author

@qinqon qinqon Mar 22, 2021

Choose a reason for hiding this comment

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

Is the default value from controller-runtime webhook server, even without /tmp/ if pod is restarted certs will be mounted too.

volumes:
- name: tls-key-pair
secret:
secretName: {{template "handlerPrefix" .}}nmstate-webhook
---
apiVersion: apps/v1
kind: Deployment
metadata:
name: {{template "handlerPrefix" .}}nmstate-cert-manager
namespace: {{ .HandlerNamespace }}
labels:
app: kubernetes-nmstate
component: kubernetes-nmstate-cert-manager
spec:
replicas: 1
strategy:
type: Recreate
selector:
matchLabels:
name: {{template "handlerPrefix" .}}nmstate-cert-manager
template:
metadata:
labels:
app: kubernetes-nmstate
component: kubernetes-nmstate-cert-manager
name: {{template "handlerPrefix" .}}nmstate-cert-manager
annotations:
description: kubernetes-nmstate-webhook rotate webhook certs
spec:
serviceAccountName: {{template "handlerPrefix" .}}nmstate-handler
nodeSelector: {{ toYaml .WebhookNodeSelector | nindent 8 }}
tolerations: {{ toYaml .WebhookTolerations | nindent 8 }}
affinity: {{ toYaml .WebhookAffinity | nindent 8 }}
containers:
- name: nmstate-cert-manager
args:
- --v=production
# Replace this with the built image name
image: {{ .HandlerImage }}
imagePullPolicy: {{ .HandlerPullPolicy }}
command:
- manager
env:
- name: WATCH_NAMESPACE
value: ""
- name: POD_NAME
valueFrom:
fieldRef:
fieldPath: metadata.name
- name: POD_NAMESPACE
valueFrom:
fieldRef:
fieldPath: metadata.namespace
- name: RUN_CERT_MANAGER
value: ""
- name: OPERATOR_NAME
value: "{{template "handlerPrefix" .}}nmstate"
- name: ENABLE_PROFILER
value: "False"
- name: PROFILER_PORT
value: "6060"
- name: CA_ROTATE_INTERVAL
value: {{ .CARotateInterval | default "8760h0m0s" }}
- name: CA_OVERLAP_INTERVAL
value: {{ .CAOverlapInterval | default "24h0m0s" }}
- name: CERT_ROTATE_INTERVAL
value: {{ .CertRotateInterval | default "4380h0m0s" }}
- name: CERT_OVERLAP_INTERVAL
value: {{ .CertOverlapInterval | default "24h0m0s" }}
---
apiVersion: apps/v1
kind: DaemonSet
metadata:
name: {{template "handlerPrefix" .}}nmstate-handler
Expand Down Expand Up @@ -187,7 +234,7 @@ spec:
publishNotReadyAddresses: true
ports:
- port: 443
targetPort: 8443
targetPort: 9443
selector:
name: {{template "handlerPrefix" .}}nmstate-webhook
---
Expand Down Expand Up @@ -261,13 +308,3 @@ spec:
selector:
matchLabels:
name: {{template "handlerPrefix" .}}nmstate-webhook
---
apiVersion: v1
kind: Secret
metadata:
name: {{template "handlerPrefix" .}}nmstate-webhook
namespace: {{ .HandlerNamespace }}
type: kubernetes.io/tls
data:
tls.crt: YmFkIGNlcnRpZmljYXRlCg==
tls.key: YmFkIGtleQo=
33 changes: 24 additions & 9 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,9 @@ func main() {
if environment.IsWebhook() {
ctrlOptions.LeaderElection = true
ctrlOptions.LeaderElectionID = "5d2e944a.nmstate.io"
} else if environment.IsCertManager() {
ctrlOptions.LeaderElection = true
ctrlOptions.LeaderElectionID = "5d2e944b.nmstate.io"
}

mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrlOptions)
Expand All @@ -101,39 +104,51 @@ func main() {
os.Exit(1)
}

// Runs only webhook controllers if it's specified
if environment.IsWebhook() {

webhookOpts := certificate.Options{
if environment.IsCertManager() {
certManagerOpts := certificate.Options{
Namespace: os.Getenv("POD_NAMESPACE"),
WebhookName: "nmstate",
WebhookType: certificate.MutatingWebhook,
}

webhookOpts.CARotateInterval, err = environment.LookupAsDuration("CA_ROTATE_INTERVAL")
certManagerOpts.CARotateInterval, err = environment.LookupAsDuration("CA_ROTATE_INTERVAL")
if err != nil {
setupLog.Error(err, "Failed retrieving ca rotate interval")
os.Exit(1)
}

webhookOpts.CAOverlapInterval, err = environment.LookupAsDuration("CA_OVERLAP_INTERVAL")
certManagerOpts.CAOverlapInterval, err = environment.LookupAsDuration("CA_OVERLAP_INTERVAL")
if err != nil {
setupLog.Error(err, "Failed retrieving ca overlap interval")
os.Exit(1)
}

webhookOpts.CertRotateInterval, err = environment.LookupAsDuration("CERT_ROTATE_INTERVAL")
certManagerOpts.CertRotateInterval, err = environment.LookupAsDuration("CERT_ROTATE_INTERVAL")
if err != nil {
setupLog.Error(err, "Failed retrieving cert rotate interval")
os.Exit(1)
}
webhookOpts.CertOverlapInterval, err = environment.LookupAsDuration("CERT_OVERLAP_INTERVAL")

certManagerOpts.CertOverlapInterval, err = environment.LookupAsDuration("CERT_OVERLAP_INTERVAL")
if err != nil {
setupLog.Error(err, "Failed retrieving cert overlap interval")
os.Exit(1)
}

if err := webhook.AddToManager(mgr, webhookOpts); err != nil {
certManager, err := certificate.NewManager(mgr.GetClient(), certManagerOpts)
if err != nil {
setupLog.Error(err, "unable to create cert-manager", "controller", "cert-manager")
os.Exit(1)
}

err = certManager.Add(mgr)
if err != nil {
setupLog.Error(err, "unable to add cert-manager to controller-runtime manager", "controller", "cert-manager")
os.Exit(1)
}
// Runs only webhook controllers if it's specified
} else if environment.IsWebhook() {
if err := webhook.AddToManager(mgr); err != nil {
setupLog.Error(err, "Cannot initialize webhook")
os.Exit(1)
}
Expand Down
12 changes: 9 additions & 3 deletions pkg/environment/environment.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,19 @@ func IsOperator() bool {

// IsWebhook returns true when RUN_WEBHOOK_SERVER env var is present
func IsWebhook() bool {
_, runOperator := os.LookupEnv("RUN_WEBHOOK_SERVER")
return runOperator
_, runWebhook := os.LookupEnv("RUN_WEBHOOK_SERVER")
return runWebhook
}

// IsCertManager return true when RUN_CERT_MANAGER env var is present
func IsCertManager() bool {
_, runCertManager := os.LookupEnv("RUN_CERT_MANAGER")
return runCertManager
}

// IsHandler returns true if it's not the operator or webhook server
func IsHandler() bool {
return !IsWebhook() && !IsOperator()
return !IsWebhook() && !IsOperator() && !IsCertManager()
}

// Returns node name runnig the pod
Expand Down
31 changes: 8 additions & 23 deletions pkg/webhook/nodenetworkconfigurationpolicy/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,40 +2,25 @@ package nodenetworkconfigurationpolicy

import (
"sigs.k8s.io/controller-runtime/pkg/manager"

"github.com/pkg/errors"

"github.com/qinqon/kube-admission-webhook/pkg/certificate"
webhookserver "github.com/qinqon/kube-admission-webhook/pkg/webhook/server"
"sigs.k8s.io/controller-runtime/pkg/webhook"
)

const (
webhookName = "nmstate"
)

func Add(mgr manager.Manager, o certificate.Options) error {

func Add(mgr manager.Manager) error {
// We need two hooks, the update of nncp and nncp/status (it's a subresource) happends
// at different times, also if you modify status at nncp webhook it does not modify it
// you need at nncp/status webhook that will catch that and do the final modifications.
// So this works this way:
// 1.- User changes nncp desiredState so it triggers deleteConditionsHook()
// 2.- Since we have delete the condition the status-mutate webhook get called and
// there we set conditions to Unknown this final result will be updated.
server, err := webhookserver.New(mgr.GetClient(), o,
webhookserver.WithHook("/nodenetworkconfigurationpolicies-mutate", deleteConditionsHook()),
webhookserver.WithHook("/nodenetworkconfigurationpolicies-status-mutate", setConditionsUnknownHook()),
webhookserver.WithHook("/nodenetworkconfigurationpolicies-timestamp-mutate", setTimestampAnnotationHook()),
webhookserver.WithHook("/nodenetworkconfigurationpolicies-progress-validate", validatePolicyUpdateHook(mgr.GetClient())),
)
if err != nil {
return errors.Wrap(err, "failed creating new webhook server")
}
return server.Add(mgr)
}

// add adds a new Webhook to mgr with r as the webhook.Server
func add(mgr manager.Manager, s manager.Runnable) error {
mgr.Add(s)
return nil
server := &webhook.Server{}
server.Register("/nodenetworkconfigurationpolicies-mutate", deleteConditionsHook())
server.Register("/nodenetworkconfigurationpolicies-status-mutate", setConditionsUnknownHook())
server.Register("/nodenetworkconfigurationpolicies-timestamp-mutate", setTimestampAnnotationHook())
server.Register("/nodenetworkconfigurationpolicies-progress-validate", validatePolicyUpdateHook(mgr.GetClient()))
return mgr.Add(server)
}
8 changes: 3 additions & 5 deletions pkg/webhook/webhook.go
Original file line number Diff line number Diff line change
@@ -1,18 +1,16 @@
package webhook

import (
"github.com/qinqon/kube-admission-webhook/pkg/certificate"

"sigs.k8s.io/controller-runtime/pkg/manager"
)

// AddToManagerFuncs is a list of functions to add all Controllers to the Manager
var AddToManagerFuncs []func(m manager.Manager, o certificate.Options) error
var AddToManagerFuncs []func(m manager.Manager) error

// AddToManager adds all Controllers to the Manager
func AddToManager(m manager.Manager, o certificate.Options) error {
func AddToManager(m manager.Manager) error {
for _, f := range AddToManagerFuncs {
if err := f(m, o); err != nil {
if err := f(m); err != nil {
return err
}
}
Expand Down