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 webhook secret and ValidatingWebhookConfiguration certificate management #2126

Merged
merged 7 commits into from
Nov 20, 2019
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
95 changes: 49 additions & 46 deletions cmd/manager/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,11 @@ import (
"strings"
"time"

"github.com/elastic/cloud-on-k8s/pkg/about"
// allow gcp authentication
_ "k8s.io/client-go/plugin/pkg/client/auth/gcp"

"github.com/elastic/cloud-on-k8s/pkg/about"
estype "github.com/elastic/cloud-on-k8s/pkg/apis/elasticsearch/v1beta1"
"github.com/elastic/cloud-on-k8s/pkg/controller/apmserver"
asesassn "github.com/elastic/cloud-on-k8s/pkg/controller/apmserverelasticsearchassociation"
"github.com/elastic/cloud-on-k8s/pkg/controller/common/certificates"
Expand All @@ -26,21 +27,18 @@ import (
kbassn "github.com/elastic/cloud-on-k8s/pkg/controller/kibanaassociation"
"github.com/elastic/cloud-on-k8s/pkg/controller/license"
licensetrial "github.com/elastic/cloud-on-k8s/pkg/controller/license/trial"
"github.com/elastic/cloud-on-k8s/pkg/controller/webhook"
"github.com/elastic/cloud-on-k8s/pkg/dev"
"github.com/elastic/cloud-on-k8s/pkg/dev/portforward"
"github.com/elastic/cloud-on-k8s/pkg/utils/net"

// TODO (sabo): re-enable when webhooks are usable
// "github.com/elastic/cloud-on-k8s/pkg/webhook"

clientgoscheme "k8s.io/client-go/kubernetes/scheme"
ctrl "sigs.k8s.io/controller-runtime"

"github.com/spf13/cobra"
"github.com/spf13/viper"
"k8s.io/client-go/kubernetes"
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/cache"
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/manager"
"sigs.k8s.io/controller-runtime/pkg/manager/signals"
)

Expand All @@ -56,10 +54,12 @@ const (
CertValidityFlag = "cert-validity"
CertRotateBeforeFlag = "cert-rotate-before"

AutoInstallWebhooksFlag = "auto-install-webhooks"
OperatorNamespaceFlag = "operator-namespace"
WebhookSecretFlag = "webhook-secret"
WebhookPodsLabelFlag = "webhook-pods-label"
OperatorNamespaceFlag = "operator-namespace"

ManageWebhookCertsFlag = "manage-webhook-certs"
WebhookSecretFlag = "webhook-secret"
WebhookConfigurationName = "elastic-webhook.k8s.elastic.co"
WebhookPort = 9443

DebugHTTPServerListenAddressFlag = "debug-http-listen"
)
Expand Down Expand Up @@ -123,20 +123,15 @@ func init() {
"Duration representing how long before expiration TLS certificates should be reissued",
)
Cmd.Flags().Bool(
AutoInstallWebhooksFlag,
ManageWebhookCertsFlag,
true,
"enables automatic webhook installation (RBAC permission for service, secret and validatingwebhookconfigurations needed)",
"enables automatic certificates management for the webhook. The Secret and the ValidatingWebhookConfiguration must be created before running the operator",
)
Cmd.Flags().String(
OperatorNamespaceFlag,
"",
"k8s namespace the operator runs in",
)
Cmd.Flags().String(
WebhookPodsLabelFlag,
"",
"k8s label to select pods running the operator",
)
Cmd.Flags().String(
WebhookSecretFlag,
"",
Expand Down Expand Up @@ -236,6 +231,7 @@ func execute() {
}
opts.MetricsBindAddress = fmt.Sprintf(":%d", metricsPort) // 0 to disable

opts.Port = WebhookPort
mgr, err := ctrl.NewManager(cfg, opts)
if err != nil {
log.Error(err, "unable to create controller manager")
Expand Down Expand Up @@ -281,6 +277,10 @@ func execute() {
},
}

if operator.HasRole(operator.WebhookServer, roles) {
barkbay marked this conversation as resolved.
Show resolved Hide resolved
setupWebhook(mgr, params.CertRotation, clientset)
}

if operator.HasRole(operator.NamespaceOperator, roles) {
if err = apmserver.Add(mgr, params); err != nil {
log.Error(err, "unable to create controller", "controller", "ApmServer")
Expand Down Expand Up @@ -314,13 +314,6 @@ func execute() {
}
}

// TODO (sabo): re-enable when webhooks are usable
// log.Info("Setting up webhooks")
// if err := webhook.AddToManager(mgr, roles, newWebhookParameters); err != nil {
// log.Error(err, "unable to register webhooks to the manager")
// os.Exit(1)
// }

log.Info("Starting the manager", "uuid", operatorInfo.OperatorUUID,
"namespace", operatorNamespace, "version", operatorInfo.BuildInfo.Version,
"build_hash", operatorInfo.BuildInfo.Hash, "build_date", operatorInfo.BuildInfo.Date,
Expand All @@ -331,26 +324,6 @@ func execute() {
}
}

// TODO (sabo): re-enable when webhooks are usable
// func newWebhookParameters() (*webhook.Parameters, error) {
// autoInstall := viper.GetBool(AutoInstallWebhooksFlag)
// ns := viper.GetString(OperatorNamespaceFlag)
// if ns == "" && autoInstall {
// return nil, fmt.Errorf("%s needs to be set for webhook auto installation", OperatorNamespaceFlag)
// }
// svcSelector := viper.GetString(WebhookPodsLabelFlag)
// sec := viper.GetString(WebhookSecretFlag)
// return &webhook.Parameters{
// Bootstrap: webhook.NewBootstrapOptions(webhook.BootstrapOptionsParams{
// Namespace: ns,
// ManagedNamespace: viper.GetString(NamespaceFlagName),
// SecretName: sec,
// ServiceSelector: svcSelector,
// }),
// AutoInstall: autoInstall,
// }, nil
// }

func ValidateCertExpirationFlags(validityFlag string, rotateBeforeFlag string) (time.Duration, time.Duration) {
certValidity := viper.GetDuration(validityFlag)
certRotateBefore := viper.GetDuration(rotateBeforeFlag)
Expand All @@ -360,3 +333,33 @@ func ValidateCertExpirationFlags(validityFlag string, rotateBeforeFlag string) (
}
return certValidity, certRotateBefore
}

func setupWebhook(mgr manager.Manager, certRotation certificates.RotationParams, clientset kubernetes.Interface) {
manageWebhookCerts := viper.GetBool(ManageWebhookCertsFlag)
if manageWebhookCerts {
log.Info("Automatic management of the webhook certificates enabled")
// Ensure that all the certificates needed by the webhook server are already created
webhookParams := webhook.Params{
Namespace: viper.GetString(OperatorNamespaceFlag),
SecretName: viper.GetString(WebhookSecretFlag),
WebhookConfigurationName: WebhookConfigurationName,
Rotation: certRotation,
}

// Force a first reconciliation to create the resources before the server is started
if err := webhookParams.ReconcileResources(clientset); err != nil {
log.Error(err, "unable to setup and fill the webhook certificates")
os.Exit(1)
}

if err := webhook.Add(mgr, webhookParams, clientset); err != nil {
log.Error(err, "unable to create controller", "controller", webhook.ControllerName)
os.Exit(1)
}
}

if err := (&estype.Elasticsearch{}).SetupWebhookWithManager(mgr); err != nil {
log.Error(err, "unable to create webhook", "webhook", "Elasticsearch")
os.Exit(1)
}
}
13 changes: 11 additions & 2 deletions config/operator/all-in-one/operator.template.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ spec:
fieldRef:
fieldPath: metadata.namespace
- name: WEBHOOK_SECRET
value: webhook-server-secret
value: elastic-webhook-server-cert
- name: WEBHOOK_PODS_LABEL
value: elastic-operator
- name: OPERATOR_IMAGE
Expand All @@ -41,7 +41,16 @@ spec:
cpu: 100m
memory: 50Mi
ports:
- containerPort: 9876
- containerPort: 9443
name: webhook-server
protocol: TCP
volumeMounts:
- mountPath: /tmp/k8s-webhook-server/serving-certs
name: cert
readOnly: true
terminationGracePeriodSeconds: 10
volumes:
- name: cert
secret:
defaultMode: 420
secretName: elastic-webhook-server-cert
1 change: 1 addition & 0 deletions config/operator/all-in-one/webhook.template.yaml
13 changes: 11 additions & 2 deletions config/operator/global/operator.template.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ spec:
fieldRef:
fieldPath: metadata.namespace
- name: WEBHOOK_SECRET
value: webhook-server-secret
value: elastic-webhook-server-cert
- name: WEBHOOK_PODS_LABEL
value: elastic-global-operator
- name: OPERATOR_IMAGE
Expand All @@ -41,8 +41,17 @@ spec:
cpu: 100m
memory: 20Mi
ports:
- containerPort: 9876
- containerPort: 9443
name: webhook-server
protocol: TCP
volumeMounts:
- mountPath: /tmp/k8s-webhook-server/serving-certs
name: cert
readOnly: true
terminationGracePeriodSeconds: 10
volumes:
Copy link
Contributor

@anyasabo anyasabo Nov 20, 2019

Choose a reason for hiding this comment

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

This is maybe overthinking it. I wonder if we want the secret to be in this file, because right now this yaml on its own will fail to deploy because the secret doesn't exist. You would need to deploy the webhook yaml also.

That might be a usability thing we can punt on in this PR though. I think right now most people will have the ability to deploy everything, and those that don't have the ability to edit yaml files. We might want to adjust in the future but this should work for now.

- name: cert
secret:
defaultMode: 420
secretName: elastic-webhook-server-cert

42 changes: 42 additions & 0 deletions config/operator/global/webhook.template.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
apiVersion: admissionregistration.k8s.io/v1beta1
kind: ValidatingWebhookConfiguration
metadata:
name: elastic-webhook.k8s.elastic.co
webhooks:
- clientConfig:
caBundle: Cg==
service:
name: elastic-webhook-server
namespace: <NAMESPACE>
# this is the path controller-runtime automatically generates
path: /validate-elasticsearch-k8s-elastic-co-v1beta1-elasticsearch
failurePolicy: Ignore
name: elastic-es-validation.k8s.elastic.co
rules:
- apiGroups:
- elasticsearch.k8s.elastic.co
apiVersions:
- v1beta1
operations:
- CREATE
- UPDATE
resources:
- elasticsearches
---
apiVersion: v1
kind: Service
metadata:
name: elastic-webhook-server
namespace: <NAMESPACE>
spec:
ports:
- port: 443
targetPort: 9443
selector:
control-plane: elastic-operator
---
apiVersion: v1
kind: Secret
metadata:
name: elastic-webhook-server-cert
namespace: <NAMESPACE>
2 changes: 1 addition & 1 deletion config/webhook/manifests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ webhooks:
namespace: system
path: /validate-elasticsearch
failurePolicy: Ignore
name: elastic-es-validation
name: elastic-es-validation.k8s.elastic.co
rules:
- apiGroups:
- elasticsearch.k8s.elastic.co
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/elasticsearch/v1beta1/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/webhook"
)

// +kubebuilder:webhook:path=/validate-elasticsearch,mutating=false,failurePolicy=ignore,groups=elasticsearch.k8s.elastic.co,resources=elasticsearches,verbs=create;update,versions=v1beta1,name=elastic-es-validation
// +kubebuilder:webhook:path=/validate-elasticsearch,mutating=false,failurePolicy=ignore,groups=elasticsearch.k8s.elastic.co,resources=elasticsearches,verbs=create;update,versions=v1beta1,name=elastic-es-validation.k8s.elastic.co

func (r *Elasticsearch) SetupWebhookWithManager(mgr ctrl.Manager) error {
return ctrl.NewWebhookManagedBy(mgr).
Expand Down
18 changes: 9 additions & 9 deletions pkg/controller/common/certificates/ca_reconcile.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,14 +70,14 @@ func ReconcileCAForOwner(
}

// build CA
ca := buildCAFromSecret(caInternalSecret)
ca := BuildCAFromSecret(caInternalSecret)
if ca == nil {
log.Info("Cannot build CA from secret, creating a new one", "owner_namespace", owner.GetNamespace(), "owner_name", owner.GetName(), "ca_type", caType)
return renewCA(cl, namer, owner, labels, rotationParams.Validity, scheme, caType)
}

// renew if cannot reuse
if !canReuseCA(ca, rotationParams.RotateBefore) {
if !CanReuseCA(ca, rotationParams.RotateBefore) {
log.Info("Cannot reuse existing CA, creating a new one", "owner_namespace", owner.GetNamespace(), "owner_name", owner.GetName(), "ca_type", caType)
return renewCA(cl, namer, owner, labels, rotationParams.Validity, scheme, caType)
}
Expand Down Expand Up @@ -125,14 +125,14 @@ func renewCA(
return ca, nil
}

// canReuseCA returns true if the given CA is valid for reuse
func canReuseCA(ca *CA, expirationSafetyMargin time.Duration) bool {
return PrivateMatchesPublicKey(ca.Cert.PublicKey, *ca.PrivateKey) && certIsValid(*ca.Cert, expirationSafetyMargin)
// CanReuseCA returns true if the given CA is valid for reuse
func CanReuseCA(ca *CA, expirationSafetyMargin time.Duration) bool {
return PrivateMatchesPublicKey(ca.Cert.PublicKey, *ca.PrivateKey) && CertIsValid(*ca.Cert, expirationSafetyMargin)
}

// certIsValid returns true if the given cert is valid,
// CertIsValid returns true if the given cert is valid,
// according to a safety time margin.
func certIsValid(cert x509.Certificate, expirationSafetyMargin time.Duration) bool {
func CertIsValid(cert x509.Certificate, expirationSafetyMargin time.Duration) bool {
now := time.Now()
if now.Before(cert.NotBefore) {
log.Info("CA cert is not valid yet", "subject", cert.Subject)
Expand Down Expand Up @@ -166,9 +166,9 @@ func internalSecretForCA(
}
}

// buildCAFromSecret parses the given secret into a CA.
// BuildCAFromSecret parses the given secret into a CA.
// It returns nil if the secrets could not be parsed into a CA.
func buildCAFromSecret(caInternalSecret corev1.Secret) *CA {
func BuildCAFromSecret(caInternalSecret corev1.Secret) *CA {
if caInternalSecret.Data == nil {
return nil
}
Expand Down
14 changes: 7 additions & 7 deletions pkg/controller/common/certificates/ca_reconcile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ func Test_certIsValid(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := certIsValid(tt.cert, tt.safetyMargin); got != tt.want {
t.Errorf("certIsValid() = %v, want %v", got, tt.want)
if got := CertIsValid(tt.cert, tt.safetyMargin); got != tt.want {
t.Errorf("CertIsValid() = %v, want %v", got, tt.want)
}
})
}
Expand Down Expand Up @@ -134,8 +134,8 @@ func Test_canReuseCA(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := canReuseCA(tt.ca(), DefaultRotateBefore); got != tt.want {
t.Errorf("canReuseCA() = %v, want %v", got, tt.want)
if got := CanReuseCA(tt.ca(), DefaultRotateBefore); got != tt.want {
t.Errorf("CanReuseCA() = %v, want %v", got, tt.want)
}
})
}
Expand All @@ -152,7 +152,7 @@ func checkCASecrets(
expectedExpiration time.Duration,
) {
// ca cert should be valid
require.True(t, certIsValid(*ca.Cert, DefaultRotateBefore))
require.True(t, CertIsValid(*ca.Cert, DefaultRotateBefore))

// expiration date should be correctly set
require.True(t, ca.Cert.NotBefore.After(time.Now().Add(-1*time.Hour)))
Expand Down Expand Up @@ -181,7 +181,7 @@ func checkCASecrets(
require.NotEmpty(t, internalCASecret.Data[KeyFileName])

// secret should be ok to parse as a CA
parsedCa := buildCAFromSecret(internalCASecret)
parsedCa := BuildCAFromSecret(internalCASecret)
require.NotNil(t, parsedCa)
// and return the ca
require.True(t, ca.Cert.Equal(parsedCa.Cert))
Expand Down Expand Up @@ -377,7 +377,7 @@ func Test_buildCAFromSecret(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ca := buildCAFromSecret(tt.internalSecret)
ca := BuildCAFromSecret(tt.internalSecret)
if !reflect.DeepEqual(ca, tt.wantCa) {
t.Errorf("CaFromSecrets() got = %v, want %v", ca, tt.wantCa)
}
Expand Down
Loading