Skip to content

Commit

Permalink
Add auth enabler (#204)
Browse files Browse the repository at this point in the history
fixes #160 

## Changelog
### Added
* [chart+kustomize]
  * what: Added get secrets permissions for operator namespace.
  * why: Necessary for operator to get secrets with certificates.
* [api]
  * what: Added helper functions to let getting settings easier.
  * why: Unify getting settings throughout the project code.
* [api]
  * what: Added ServerTrustedCASecret to spec.
* why: Necessary to mount this certificate to let operator trust etcd
cluster.
* **[controller]**
* **what: Added functionality to disable and enable auth, add root role,
root user.**
  * **why: Necessary for the customer.**

### Changed
* [api]
  * what: Adjusted field descriptions for security fields.
* why: Necessary to let customers know where we expect created secrets
with certificates.
* [etcdcluster_controller_test]
  * what: Commented autotests that reconcile twice and set sts ready.
* why: Not clear how to handle failed tests. It is not supposed to set
ready status when it is not ready. Creating mocks for every function
that uses etcdClient will take much time in the future
  • Loading branch information
Kirill-Garbar authored Jun 13, 2024
1 parent c0cb38b commit 9978183
Show file tree
Hide file tree
Showing 20 changed files with 889 additions and 125 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ vendor

# editor and IDE paraphernalia
.idea
.vscode
*.swp
*.swo
*~
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ mod-tidy: ## Run go mod tidy against code.

.PHONY: test
test: manifests generate fmt vet envtest ## Run tests.
echo "Check for kubernetes version $(K8S_VERSION_TRIMMED_V) in $(ENVTEST)"
@echo "Check for kubernetes version $(K8S_VERSION_TRIMMED_V) in $(ENVTEST)"
@$(ENVTEST) list | grep -q $(K8S_VERSION_TRIMMED_V)
KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(K8S_VERSION_TRIMMED_V) --bin-dir $(LOCALBIN) -p path)" go test $$(go list ./... | grep -v /e2e) -coverprofile cover.out

Expand Down
28 changes: 26 additions & 2 deletions api/v1alpha1/etcdcluster_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,18 @@ func (r *EtcdCluster) CalculateQuorumSize() int {
return int(*r.Spec.Replicas)/2 + 1
}

func (c *EtcdCluster) IsClientSecurityEnabled() bool {
return c.Spec.Security != nil && c.Spec.Security.TLS.ClientSecret != ""
}

func (c *EtcdCluster) IsServerSecurityEnabled() bool {
return c.Spec.Security != nil && c.Spec.Security.TLS.ServerSecret != ""
}

func (c *EtcdCluster) IsServerTrustedCADefined() bool {
return c.Spec.Security != nil && c.Spec.Security.TLS.ServerTrustedCASecret != ""
}

// +kubebuilder:object:root=true

// EtcdClusterList contains a list of EtcdCluster
Expand Down Expand Up @@ -174,24 +186,36 @@ type SecuritySpec struct {
// Section for user-managed tls certificates
// +optional
TLS TLSSpec `json:"tls,omitempty"`
// Section to enable etcd auth
EnableAuth bool `json:"enableAuth,omitempty"`
}

// TLSSpec defines user-managed certificates names.
type TLSSpec struct {
// Trusted CA certificate secret to secure peer-to-peer communication between etcd nodes. It is expected to have tls.crt field in the secret.
// Trusted CA certificate secret to secure peer-to-peer communication between etcd nodes. It is expected to have ca.crt field in the secret.
// This secret must be created in the namespace with etcdCluster CR.
// +optional
PeerTrustedCASecret string `json:"peerTrustedCASecret,omitempty"`
// Certificate secret to secure peer-to-peer communication between etcd nodes. It is expected to have tls.crt and tls.key fields in the secret.
// This secret must be created in the namespace with etcdCluster CR.
// +optional
PeerSecret string `json:"peerSecret,omitempty"`
// Trusted CA for etcd server certificates for client-server communication. Is necessary to set trust between operator and etcd.
// It is expected to have ca.crt field in the secret. If it is not specified, then insecure communication will be used.
// This secret must be created in the namespace with etcdCluster CR.
// +optional
ServerTrustedCASecret string `json:"serverTrustedCASecret,omitempty"`
// Server certificate secret to secure client-server communication. Is provided to the client who connects to etcd by client port (2379 by default).
// It is expected to have tls.crt and tls.key fields in the secret.
// This secret must be created in the namespace with etcdCluster CR.
// +optional
ServerSecret string `json:"serverSecret,omitempty"`
// Trusted CA for client certificates that are provided by client to etcd. It is expected to have tls.crt field in the secret.
// Trusted CA for client certificates that are provided by client to etcd. It is expected to have ca.crt field in the secret.
// This secret must be created in the namespace with etcdCluster CR.
// +optional
ClientTrustedCASecret string `json:"clientTrustedCASecret,omitempty"`
// Client certificate for etcd-operator to do maintenance. It is expected to have tls.crt and tls.key fields in the secret.
// This secret must be created in the namespace with etcdCluster CR.
// +optional
ClientSecret string `json:"clientSecret,omitempty"`
}
Expand Down
102 changes: 102 additions & 0 deletions api/v1alpha1/etcdcluster_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,105 @@ var _ = Context("CalculateQuorumSize", func() {
Expect(etcdCluster.CalculateQuorumSize()).To(Equal(3))
})
})

var _ = Describe("Aux Functions", func() {

Context("When running IsClientSecurityEnabled function", func() {
It("should return true if ClientSecret is set", func() {
cluster := EtcdCluster{
Spec: EtcdClusterSpec{
Security: &SecuritySpec{
TLS: TLSSpec{
ClientSecret: "some-client-secret",
},
},
},
}
Expect(cluster.IsClientSecurityEnabled()).To(BeTrue())
})

It("should return false if ClientSecret is not set", func() {
cluster := EtcdCluster{
Spec: EtcdClusterSpec{
Security: &SecuritySpec{
TLS: TLSSpec{},
},
},
}
Expect(cluster.IsClientSecurityEnabled()).To(BeFalse())
})

It("should return false if Security is nil", func() {
cluster := &EtcdCluster{
Spec: EtcdClusterSpec{},
}
Expect(cluster.IsClientSecurityEnabled()).To(BeFalse())
})
})

Context("When running IsServerSecurityEnabled function", func() {
It("should return true if ServerSecret is set", func() {
cluster := &EtcdCluster{
Spec: EtcdClusterSpec{
Security: &SecuritySpec{
TLS: TLSSpec{
ServerSecret: "some-server-secret",
},
},
},
}
Expect(cluster.IsServerSecurityEnabled()).To(BeTrue())
})

It("should return false if ServerSecret is not set", func() {
cluster := &EtcdCluster{
Spec: EtcdClusterSpec{
Security: &SecuritySpec{
TLS: TLSSpec{},
},
},
}
Expect(cluster.IsServerSecurityEnabled()).To(BeFalse())
})

It("should return false if Security is nil", func() {
cluster := &EtcdCluster{
Spec: EtcdClusterSpec{},
}
Expect(cluster.IsServerSecurityEnabled()).To(BeFalse())
})
})

Context("When running IsServerTrustedCADefined function", func() {
It("should return true if ServerTrustedCASecret is set", func() {
cluster := &EtcdCluster{
Spec: EtcdClusterSpec{
Security: &SecuritySpec{
TLS: TLSSpec{
ServerTrustedCASecret: "some-ca-secret",
},
},
},
}
Expect(cluster.IsServerTrustedCADefined()).To(BeTrue())
})

It("should return false if ServerTrustedCASecret is not set", func() {
cluster := &EtcdCluster{
Spec: EtcdClusterSpec{
Security: &SecuritySpec{
TLS: TLSSpec{},
},
},
}
Expect(cluster.IsServerTrustedCADefined()).To(BeFalse())
})

It("should return false if Security is nil", func() {
cluster := &EtcdCluster{
Spec: EtcdClusterSpec{},
}
Expect(cluster.IsServerTrustedCADefined()).To(BeFalse())
})
})
})
9 changes: 9 additions & 0 deletions api/v1alpha1/etcdcluster_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,15 @@ func (r *EtcdCluster) validateSecurity() field.ErrorList {
)
}

if security.EnableAuth && (security.TLS.ClientSecret == "" || security.TLS.ServerSecret == "") {

allErrors = append(allErrors, field.Invalid(
field.NewPath("spec", "security"),
security.TLS,
"if auth is enabled, client secret and server secret must be provided"),
)
}

if len(allErrors) > 0 {
return allErrors
}
Expand Down
60 changes: 60 additions & 0 deletions api/v1alpha1/etcdcluster_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,66 @@ var _ = Describe("EtcdCluster Webhook", func() {
}
}
})

It("Shouldn't reject if auth is enabled and security client certs are defined", func() {
localCluster := etcdCluster.DeepCopy()
localCluster.Spec.Security = &SecuritySpec{
EnableAuth: true,
TLS: TLSSpec{
ClientTrustedCASecret: "test-client-trusted-ca-cert",
ClientSecret: "test-client-cert",
ServerSecret: "test-server-cert",
},
}
err := localCluster.validateSecurity()
Expect(err).To(BeNil())
})

It("Should reject if auth is enabled and one of client and server certs is defined", func() {
localCluster := etcdCluster.DeepCopy()
localCluster.Spec.Security = &SecuritySpec{
EnableAuth: true,
TLS: TLSSpec{
ClientSecret: "test-client-cert",
ClientTrustedCASecret: "test-client-trusted-ca-cert",
},
}
err := localCluster.validateSecurity()
if Expect(err).NotTo(BeNil()) {
expectedFieldErr := field.Invalid(
field.NewPath("spec", "security"),
localCluster.Spec.Security.TLS,
"if auth is enabled, client secret and server secret must be provided",
)
if Expect(err).To(HaveLen(1)) {
Expect(*(err[0])).To(Equal(*expectedFieldErr))
}
}

})

It("Should reject if auth is enabled and one of client and server certs is defined", func() {
localCluster := etcdCluster.DeepCopy()
localCluster.Spec.Security = &SecuritySpec{
EnableAuth: true,
TLS: TLSSpec{
ServerSecret: "test-server-cert",
},
}
err := localCluster.validateSecurity()
if Expect(err).NotTo(BeNil()) {
expectedFieldErr := field.Invalid(
field.NewPath("spec", "security"),
localCluster.Spec.Security.TLS,
"if auth is enabled, client secret and server secret must be provided",
)
if Expect(err).To(HaveLen(1)) {
Expect(*(err[0])).To(Equal(*expectedFieldErr))
}
}

})

})

Context("Validate PDB", func() {
Expand Down
26 changes: 22 additions & 4 deletions charts/etcd-operator/crds/etcd-cluster.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -202,25 +202,43 @@ spec:
security:
description: Security describes security settings of etcd (authentication, certificates, rbac)
properties:
enableAuth:
description: Section to enable etcd auth
type: boolean
tls:
description: Section for user-managed tls certificates
properties:
clientSecret:
description: Client certificate for etcd-operator to do maintenance. It is expected to have tls.crt and tls.key fields in the secret.
description: |-
Client certificate for etcd-operator to do maintenance. It is expected to have tls.crt and tls.key fields in the secret.
This secret must be created in the namespace with etcdCluster CR.
type: string
clientTrustedCASecret:
description: Trusted CA for client certificates that are provided by client to etcd. It is expected to have tls.crt field in the secret.
description: |-
Trusted CA for client certificates that are provided by client to etcd. It is expected to have ca.crt field in the secret.
This secret must be created in the namespace with etcdCluster CR.
type: string
peerSecret:
description: Certificate secret to secure peer-to-peer communication between etcd nodes. It is expected to have tls.crt and tls.key fields in the secret.
description: |-
Certificate secret to secure peer-to-peer communication between etcd nodes. It is expected to have tls.crt and tls.key fields in the secret.
This secret must be created in the namespace with etcdCluster CR.
type: string
peerTrustedCASecret:
description: Trusted CA certificate secret to secure peer-to-peer communication between etcd nodes. It is expected to have tls.crt field in the secret.
description: |-
Trusted CA certificate secret to secure peer-to-peer communication between etcd nodes. It is expected to have ca.crt field in the secret.
This secret must be created in the namespace with etcdCluster CR.
type: string
serverSecret:
description: |-
Server certificate secret to secure client-server communication. Is provided to the client who connects to etcd by client port (2379 by default).
It is expected to have tls.crt and tls.key fields in the secret.
This secret must be created in the namespace with etcdCluster CR.
type: string
serverTrustedCASecret:
description: |-
Trusted CA for etcd server certificates for client-server communication. Is necessary to set trust between operator and etcd.
It is expected to have ca.crt field in the secret. If it is not specified, then insecure communication will be used.
This secret must be created in the namespace with etcdCluster CR.
type: string
type: object
type: object
Expand Down
6 changes: 6 additions & 0 deletions charts/etcd-operator/templates/workload/deployment.yml
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,12 @@ spec:
- configMapRef:
name: {{ include "etcd-operator.fullname" . }}-env
{{- end }}
env:
- name: POD_NAMESPACE
valueFrom:
fieldRef:
apiVersion: v1
fieldPath: metadata.namespace
volumeMounts:
- mountPath: /tmp/k8s-webhook-server/serving-certs
name: cert
Expand Down
26 changes: 22 additions & 4 deletions config/crd/bases/etcd.aenix.io_etcdclusters.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -192,25 +192,43 @@ spec:
security:
description: Security describes security settings of etcd (authentication, certificates, rbac)
properties:
enableAuth:
description: Section to enable etcd auth
type: boolean
tls:
description: Section for user-managed tls certificates
properties:
clientSecret:
description: Client certificate for etcd-operator to do maintenance. It is expected to have tls.crt and tls.key fields in the secret.
description: |-
Client certificate for etcd-operator to do maintenance. It is expected to have tls.crt and tls.key fields in the secret.
This secret must be created in the namespace with etcdCluster CR.
type: string
clientTrustedCASecret:
description: Trusted CA for client certificates that are provided by client to etcd. It is expected to have tls.crt field in the secret.
description: |-
Trusted CA for client certificates that are provided by client to etcd. It is expected to have ca.crt field in the secret.
This secret must be created in the namespace with etcdCluster CR.
type: string
peerSecret:
description: Certificate secret to secure peer-to-peer communication between etcd nodes. It is expected to have tls.crt and tls.key fields in the secret.
description: |-
Certificate secret to secure peer-to-peer communication between etcd nodes. It is expected to have tls.crt and tls.key fields in the secret.
This secret must be created in the namespace with etcdCluster CR.
type: string
peerTrustedCASecret:
description: Trusted CA certificate secret to secure peer-to-peer communication between etcd nodes. It is expected to have tls.crt field in the secret.
description: |-
Trusted CA certificate secret to secure peer-to-peer communication between etcd nodes. It is expected to have ca.crt field in the secret.
This secret must be created in the namespace with etcdCluster CR.
type: string
serverSecret:
description: |-
Server certificate secret to secure client-server communication. Is provided to the client who connects to etcd by client port (2379 by default).
It is expected to have tls.crt and tls.key fields in the secret.
This secret must be created in the namespace with etcdCluster CR.
type: string
serverTrustedCASecret:
description: |-
Trusted CA for etcd server certificates for client-server communication. Is necessary to set trust between operator and etcd.
It is expected to have ca.crt field in the secret. If it is not specified, then insecure communication will be used.
This secret must be created in the namespace with etcdCluster CR.
type: string
type: object
type: object
Expand Down
1 change: 1 addition & 0 deletions config/default/manager_auth_proxy_patch.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,4 @@ spec:
- "--health-probe-bind-address=:8081"
- "--metrics-bind-address=127.0.0.1:8080"
- "--leader-elect"
- "--log-level=debug"
6 changes: 6 additions & 0 deletions config/manager/manager.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -99,5 +99,11 @@ spec:
requests:
cpu: 10m
memory: 64Mi
env:
- name: POD_NAMESPACE
valueFrom:
fieldRef:
apiVersion: v1
fieldPath: metadata.namespace
serviceAccountName: controller-manager
terminationGracePeriodSeconds: 10
Loading

0 comments on commit 9978183

Please sign in to comment.