Don't rely on buggy metaObject Kind (elastic#1324)

* Don't rely on buggy metaObject Kind A bug in our client implementation may clear the object's Kind on certain scenarios. See kubernetes-sigs/controller-runtime#406. Let's avoid that by fixing a constant Kind returned by a method call on the resource.
sebgl · Jul 24, 2019 · 8fd649d · 8fd649d
1 parent f8f9ed3
commit 8fd649d
Show file tree

Hide file tree

Showing 9 changed files with 44 additions and 12 deletions.
diff --git a/operators/pkg/apis/apm/v1alpha1/apmserver_types.go b/operators/pkg/apis/apm/v1alpha1/apmserver_types.go
@@ -10,7 +10,10 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-const APMServerContainerName = "apm-server"
+const (
+	APMServerContainerName = "apm-server"
+	Kind                   = "ApmServer"
+)
 
 // ApmServerSpec defines the desired state of ApmServer
 type ApmServerSpec struct {
@@ -155,3 +158,9 @@ func (as *ApmServer) ElasticsearchAuth() commonv1alpha1.ElasticsearchAuth {
 func (as *ApmServer) SecureSettings() *commonv1alpha1.SecretRef {
 	return as.Spec.SecureSettings
 }
+
+// Kind can technically be retrieved from metav1.Object, but there is a bug preventing us to retrieve it
+// see https://github.com/kubernetes-sigs/controller-runtime/issues/406
+func (as *ApmServer) Kind() string {
+	return Kind
+}
diff --git a/operators/pkg/apis/elasticsearch/v1alpha1/elasticsearch_types.go b/operators/pkg/apis/elasticsearch/v1alpha1/elasticsearch_types.go
@@ -10,7 +10,10 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-const ElasticsearchContainerName = "elasticsearch"
+const (
+	ElasticsearchContainerName = "elasticsearch"
+	Kind                       = "Elasticsearch"
+)
 
 // ElasticsearchSpec defines the desired state of Elasticsearch
 type ElasticsearchSpec struct {
@@ -265,6 +268,12 @@ func (e Elasticsearch) SecureSettings() *commonv1alpha1.SecretRef {
 	return e.Spec.SecureSettings
 }
 
+// Kind can technically be retrieved from metav1.Object, but there is a bug preventing us to retrieve it
+// see https://github.com/kubernetes-sigs/controller-runtime/issues/406
+func (e Elasticsearch) Kind() string {
+	return Kind
+}
+
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
 
 // ElasticsearchList contains a list of Elasticsearch clusters

diff --git a/operators/pkg/apis/kibana/v1alpha1/kibana_types.go b/operators/pkg/apis/kibana/v1alpha1/kibana_types.go
@@ -11,7 +11,10 @@ import (
 	commonv1alpha1 "github.com/elastic/cloud-on-k8s/operators/pkg/apis/common/v1alpha1"
 )
 
-const KibanaContainerName = "kibana"
+const (
+	KibanaContainerName = "kibana"
+	Kind                = "Kibana"
+)
 
 // KibanaSpec defines the desired state of Kibana
 type KibanaSpec struct {
@@ -112,6 +115,12 @@ func (k *Kibana) SecureSettings() *commonv1alpha1.SecretRef {
 	return k.Spec.SecureSettings
 }
 
+// Kind can technically be retrieved from metav1.Object, but there is a bug preventing us to retrieve it
+// see https://github.com/kubernetes-sigs/controller-runtime/issues/406
+func (k *Kibana) Kind() string {
+	return Kind
+}
+
 // +genclient
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
 

diff --git a/operators/pkg/controller/apmserver/apmserver_controller.go b/operators/pkg/controller/apmserver/apmserver_controller.go
@@ -452,6 +452,6 @@ func (r *ReconcileApmServer) updateStatus(state State) (reconcile.Result, error)
 // finalizersFor returns the list of finalizers applying to a given APM deployment
 func (r *ReconcileApmServer) finalizersFor(as apmv1alpha1.ApmServer) []finalizer.Finalizer {
 	return []finalizer.Finalizer{
-		keystore.Finalizer(k8s.ExtractNamespacedName(&as), r.dynamicWatches, "apmserver"),
+		keystore.Finalizer(k8s.ExtractNamespacedName(&as), r.dynamicWatches, as.Kind()),
 	}
 }
diff --git a/operators/pkg/controller/apmserver/pod.go b/operators/pkg/controller/apmserver/pod.go
@@ -117,7 +117,7 @@ func newPodSpec(as *v1alpha1.ApmServer, p PodSpecParams) corev1.PodTemplateSpec
 
 	if p.keystoreResources != nil {
 		dataVolume := keystore.DataVolume(
-			strings.ToLower(as.Kind),
+			strings.ToLower(as.Kind()),
 			DataVolumePath,
 		)
 		builder.WithInitContainers(p.keystoreResources.InitContainer).

diff --git a/operators/pkg/controller/common/keystore/resources.go b/operators/pkg/controller/common/keystore/resources.go
@@ -7,14 +7,15 @@ package keystore
 import (
 	"strings"
 
-	commonv1alpha1 "github.com/elastic/cloud-on-k8s/operators/pkg/apis/common/v1alpha1"
-	"github.com/elastic/cloud-on-k8s/operators/pkg/controller/common/watches"
-	"github.com/elastic/cloud-on-k8s/operators/pkg/utils/k8s"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/tools/record"
 	logf "sigs.k8s.io/controller-runtime/pkg/runtime/log"
+
+	commonv1alpha1 "github.com/elastic/cloud-on-k8s/operators/pkg/apis/common/v1alpha1"
+	"github.com/elastic/cloud-on-k8s/operators/pkg/controller/common/watches"
+	"github.com/elastic/cloud-on-k8s/operators/pkg/utils/k8s"
 )
 
 var log = logf.Log.WithName("keystore")
@@ -35,6 +36,9 @@ type HasKeystore interface {
 	metav1.Object
 	runtime.Object
 	SecureSettings() *commonv1alpha1.SecretRef
+	// Kind can technically be retrieved from metav1.Object, but there is a bug preventing us to retrieve it
+	// see https://github.com/kubernetes-sigs/controller-runtime/issues/406
+	Kind() string
 }
 
 // NewResources optionally returns a volume and init container to include in pods,
@@ -60,7 +64,7 @@ func NewResources(
 	// build an init container to create the keystore from the secure settings volume
 	initContainer, err := initContainer(
 		*secretVolume,
-		strings.ToLower(hasKeystore.GetObjectKind().GroupVersionKind().Kind),
+		strings.ToLower(hasKeystore.Kind()),
 		initContainerParams,
 	)
 	if err != nil {

diff --git a/operators/pkg/controller/common/keystore/user_secret.go b/operators/pkg/controller/common/keystore/user_secret.go
@@ -6,6 +6,7 @@ package keystore
 
 import (
 	"fmt"
+	"strings"
 
 	commonv1alpha1 "github.com/elastic/cloud-on-k8s/operators/pkg/apis/common/v1alpha1"
 	"github.com/elastic/cloud-on-k8s/operators/pkg/controller/common/events"
@@ -114,7 +115,7 @@ func watchSecureSettings(watched watches.DynamicWatches, secureSettingsRef *comm
 // Finalizer removes any dynamic watches on external user created secret.
 func Finalizer(namespacedName types.NamespacedName, watched watches.DynamicWatches, kind string) finalizer.Finalizer {
 	return finalizer.Finalizer{
-		Name: "secure-settings.finalizers." + kind + ".k8s.elastic.co",
+		Name: "secure-settings.finalizers." + strings.ToLower(kind) + ".k8s.elastic.co",
 		Execute: func() error {
 			watched.Secrets.RemoveHandlerForKey(secureSettingsWatchName(namespacedName))
 			return nil

diff --git a/operators/pkg/controller/elasticsearch/elasticsearch_controller.go b/operators/pkg/controller/elasticsearch/elasticsearch_controller.go
@@ -294,7 +294,7 @@ func (r *ReconcileElasticsearch) finalizersFor(
 	return []finalizer.Finalizer{
 		reconciler.ExpectationsFinalizer(clusterName, r.podsExpectations),
 		r.esObservers.Finalizer(clusterName),
-		keystore.Finalizer(k8s.ExtractNamespacedName(&es), r.dynamicWatches, "elasticsearch"),
+		keystore.Finalizer(k8s.ExtractNamespacedName(&es), r.dynamicWatches, es.Kind()),
 		http.DynamicWatchesFinalizer(r.dynamicWatches, es.Name, esname.ESNamer),
 	}
 }
diff --git a/operators/pkg/controller/kibana/kibana_controller.go b/operators/pkg/controller/kibana/kibana_controller.go
@@ -216,6 +216,6 @@ func (r *ReconcileKibana) updateStatus(state State) error {
 func (r *ReconcileKibana) finalizersFor(kb kibanav1alpha1.Kibana) []finalizer.Finalizer {
 	return []finalizer.Finalizer{
 		secretWatchFinalizer(kb, r.dynamicWatches),
-		keystore.Finalizer(k8s.ExtractNamespacedName(&kb), r.dynamicWatches, "kibana"),
+		keystore.Finalizer(k8s.ExtractNamespacedName(&kb), r.dynamicWatches, kb.Kind()),
 	}
 }