APM: remove "output" element and add elasticsearchRef (#1345) (#1354)

pebrc · web-flow · commit f9c01e8d03a9 · 2019-07-23T23:22:50.000+02:00
diff --git a/operators/config/crds/apm_v1alpha1_apmserver.yaml b/operators/config/crds/apm_v1alpha1_apmserver.yaml
@@ -50,6 +50,60 @@ spec:
             config:
               description: Config represents the APM configuration.
               type: object
+            elasticsearch:
+              description: Elasticsearch configures how the APM server connects to
+                Elasticsearch
+              properties:
+                auth:
+                  description: Auth configures authentication for APM Server to use.
+                  properties:
+                    inline:
+                      description: Inline is auth provided as plaintext inline credentials.
+                      properties:
+                        password:
+                          description: Password is the password to use.
+                          type: string
+                        username:
+                          description: User is the username to use.
+                          type: string
+                      required:
+                      - username
+                      - password
+                      type: object
+                    secret:
+                      description: SecretKeyRef is a secret that contains the credentials
+                        to use.
+                      type: object
+                  type: object
+                hosts:
+                  description: Hosts are the URLs of the output Elasticsearch nodes.
+                  items:
+                    type: string
+                  type: array
+                ssl:
+                  description: SSL configures TLS-related configuration for Elasticsearch
+                  properties:
+                    certificateAuthorities:
+                      description: CertificateAuthorities is a secret that contains
+                        a `tls.crt` entry that contain certificates for server verifications.
+                      properties:
+                        secretName:
+                          type: string
+                      type: object
+                  type: object
+              type: object
+            elasticsearchRef:
+              description: ElasticsearchRef references an Elasticsearch resource in
+                the Kubernetes cluster. If the namespace is not specified, the current
+                resource namespace will be used.
+              properties:
+                name:
+                  type: string
+                namespace:
+                  type: string
+              required:
+              - name
+              type: object
             featureFlags:
               description: FeatureFlags are apm-specific flags that enable or disable
                 specific experimental features
@@ -114,65 +168,6 @@ spec:
                 must have.
               format: int32
               type: integer
-            output:
-              properties:
-                elasticsearch:
-                  description: Elasticsearch configures the Elasticsearch output
-                  properties:
-                    auth:
-                      description: Auth configures authentication for APM Server to
-                        use.
-                      properties:
-                        inline:
-                          description: Inline is auth provided as plaintext inline
-                            credentials.
-                          properties:
-                            password:
-                              description: Password is the password to use.
-                              type: string
-                            username:
-                              description: User is the username to use.
-                              type: string
-                          required:
-                          - username
-                          - password
-                          type: object
-                        secret:
-                          description: SecretKeyRef is a secret that contains the
-                            credentials to use.
-                          type: object
-                      type: object
-                    hosts:
-                      description: Hosts are the URLs of the output Elasticsearch
-                        nodes.
-                      items:
-                        type: string
-                      type: array
-                    ref:
-                      description: ElasticsearchRef allows users to reference a Elasticsearch
-                        cluster inside k8s to automatically derive the other fields.
-                      properties:
-                        name:
-                          type: string
-                        namespace:
-                          type: string
-                      required:
-                      - name
-                      type: object
-                    ssl:
-                      description: SSL configures TLS-related configuration for Elasticsearch
-                      properties:
-                        certificateAuthorities:
-                          description: CertificateAuthorities is a secret that contains
-                            a `tls.crt` entry that contain certificates for server
-                            verifications.
-                          properties:
-                            secretName:
-                              type: string
-                          type: object
-                      type: object
-                  type: object
-              type: object
             podTemplate:
               description: PodTemplate can be used to propagate configuration to APM
                 Server pods. This allows specifying custom annotations, labels, environment
diff --git a/operators/config/samples/apm/apm_es_kibana.yaml b/operators/config/samples/apm/apm_es_kibana.yaml
@@ -16,11 +16,8 @@ metadata:
 spec:
   version: "7.2.0"
   nodeCount: 1
-  output:
-    elasticsearch:
-      ref:
-        name: elasticsearch-sample
-        namespace: default
+  elasticsearchRef:
+    name: "elasticsearch-sample"
 ---
 apiVersion: kibana.k8s.elastic.co/v1alpha1
 kind: Kibana
diff --git a/operators/pkg/apis/apm/v1alpha1/apmserver_types.go b/operators/pkg/apis/apm/v1alpha1/apmserver_types.go
@@ -29,8 +29,13 @@ type ApmServerSpec struct {
 	// HTTP contains settings for HTTP.
 	HTTP commonv1alpha1.HTTPConfig `json:"http,omitempty"`
 
+	// ElasticsearchRef references an Elasticsearch resource in the Kubernetes cluster.
+	// If the namespace is not specified, the current resource namespace will be used.
+	ElasticsearchRef commonv1alpha1.ObjectSelector `json:"elasticsearchRef,omitempty"`
+
+	// Elasticsearch configures how the APM server connects to Elasticsearch
 	// +optional
-	Output Output `json:"output,omitempty"`
+	Elasticsearch ElasticsearchOutput `json:"elasticsearch,omitempty"`
 
 	// PodTemplate can be used to propagate configuration to APM Server pods.
 	// This allows specifying custom annotations, labels, environment variables,
@@ -49,17 +54,8 @@ type ApmServerSpec struct {
 	FeatureFlags commonv1alpha1.FeatureFlags `json:"featureFlags,omitempty"`
 }
 
-// Output contains output configuration for supported outputs
-type Output struct {
-	// Elasticsearch configures the Elasticsearch output
-	// +optional
-	Elasticsearch ElasticsearchOutput `json:"elasticsearch,omitempty"`
-}
-
 // Elasticsearch contains configuration for the Elasticsearch output
 type ElasticsearchOutput struct {
-	// ElasticsearchRef allows users to reference a Elasticsearch cluster inside k8s to automatically derive the other fields.
-	ElasticsearchRef *commonv1alpha1.ObjectSelector `json:"ref,omitempty"`
 
 	// Hosts are the URLs of the output Elasticsearch nodes.
 	Hosts []string `json:"hosts,omitempty"`
@@ -153,7 +149,7 @@ func (as *ApmServer) IsMarkedForDeletion() bool {
 }
 
 func (as *ApmServer) ElasticsearchAuth() commonv1alpha1.ElasticsearchAuth {
-	return as.Spec.Output.Elasticsearch.Auth
+	return as.Spec.Elasticsearch.Auth
 }
 
 func (as *ApmServer) SecureSettings() *commonv1alpha1.SecretRef {
diff --git a/operators/pkg/apis/apm/v1alpha1/zz_generated.deepcopy.go b/operators/pkg/apis/apm/v1alpha1/zz_generated.deepcopy.go
diff --git a/operators/pkg/controller/apmserver/apmserver_controller.go b/operators/pkg/controller/apmserver/apmserver_controller.go
@@ -309,7 +309,7 @@ func (r *ReconcileApmServer) deploymentParams(
 		_, _ = configChecksum.Write([]byte(params.keystoreResources.Version))
 	}
 
-	esCASecretName := as.Spec.Output.Elasticsearch.SSL.CertificateAuthorities.SecretName
+	esCASecretName := as.Spec.Elasticsearch.SSL.CertificateAuthorities.SecretName
 	if esCASecretName != "" {
 		// TODO: use apmServerCa to generate cert for deployment
 
diff --git a/operators/pkg/controller/apmserver/config/config.go b/operators/pkg/controller/apmserver/config/config.go
@@ -45,15 +45,15 @@ func NewConfigFromSpec(c k8s.Client, as v1alpha1.ApmServer) (*settings.Canonical
 	}
 
 	outputCfg := settings.NewCanonicalConfig()
-	if as.Spec.Output.Elasticsearch.IsConfigured() {
+	if as.Spec.Elasticsearch.IsConfigured() {
 		// Get username and password
 		username, password, err := association.ElasticsearchAuthSettings(c, &as)
 		if err != nil {
 			return nil, err
 		}
 		outputCfg = settings.MustCanonicalConfig(
 			map[string]interface{}{
-				"output.elasticsearch.hosts":                       as.Spec.Output.Elasticsearch.Hosts,
+				"output.elasticsearch.hosts":                       as.Spec.Elasticsearch.Hosts,
 				"output.elasticsearch.username":                    username,
 				"output.elasticsearch.password":                    password,
 				"output.elasticsearch.ssl.certificate_authorities": []string{filepath.Join(CertificatesDir, certificates.CertFileName)},
diff --git a/operators/pkg/controller/apmserverelasticsearchassociation/apmserverelasticsearchassociation_controller.go b/operators/pkg/controller/apmserverelasticsearchassociation/apmserverelasticsearchassociation_controller.go
@@ -208,13 +208,16 @@ func resultFromStatus(status commonv1alpha1.AssociationStatus) reconcile.Result
 }
 
 func (r *ReconcileApmServerElasticsearchAssociation) reconcileInternal(apmServer apmtype.ApmServer) (commonv1alpha1.AssociationStatus, error) {
-	assocKey := k8s.ExtractNamespacedName(&apmServer)
 	// no auto-association nothing to do
-	elasticsearchRef := apmServer.Spec.Output.Elasticsearch.ElasticsearchRef
-	if elasticsearchRef == nil {
+	elasticsearchRef := apmServer.Spec.ElasticsearchRef
+	if !elasticsearchRef.IsDefined() {
 		return "", nil
 	}
-
+	if elasticsearchRef.Namespace == "" {
+		// no namespace provided: default to the APM server namespace
+		elasticsearchRef.Namespace = apmServer.Namespace
+	}
+	assocKey := k8s.ExtractNamespacedName(&apmServer)
 	// Make sure we see events from Elasticsearch using a dynamic watch
 	// will become more relevant once we refactor user handling to CRDs and implement
 	// syncing of user credentials across namespaces
@@ -244,8 +247,6 @@ func (r *ReconcileApmServerElasticsearchAssociation) reconcileInternal(apmServer
 	}
 
 	var expectedEsConfig apmtype.ElasticsearchOutput
-	expectedEsConfig.ElasticsearchRef = apmServer.Spec.Output.Elasticsearch.ElasticsearchRef
-
 	// TODO: look up public certs secret name from the ES cluster resource instead of relying on naming convention
 	var publicCertsSecret corev1.Secret
 	publicCertsSecretKey := http.PublicCertsSecretRef(
@@ -261,8 +262,8 @@ func (r *ReconcileApmServerElasticsearchAssociation) reconcileInternal(apmServer
 	expectedEsConfig.Auth.SecretKeyRef = clearTextSecretKeySelector(apmServer)
 
 	// TODO: this is a bit rough
-	if !reflect.DeepEqual(apmServer.Spec.Output.Elasticsearch, expectedEsConfig) {
-		apmServer.Spec.Output.Elasticsearch = expectedEsConfig
+	if !reflect.DeepEqual(apmServer.Spec.Elasticsearch, expectedEsConfig) {
+		apmServer.Spec.Elasticsearch = expectedEsConfig
 		log.Info("Updating Apm Server spec with Elasticsearch output configuration", "namespace", apmServer.Namespace, "as_name", apmServer.Name)
 		if err := r.Update(&apmServer); err != nil {
 			return commonv1alpha1.AssociationPending, err
@@ -289,7 +290,7 @@ func deleteOrphanedResources(c k8s.Client, apm apmtype.ApmServer) error {
 
 	for _, s := range secrets.Items {
 		controlledBy := metav1.IsControlledBy(&s, &apm)
-		if controlledBy && !apm.Spec.Output.Elasticsearch.ElasticsearchRef.IsDefined() {
+		if controlledBy && !apm.Spec.ElasticsearchRef.IsDefined() {
 			log.Info("Deleting secret", "namespace", s.Namespace, "secret_name", s.Name, "as_name", apm.Name)
 			if err := c.Delete(&s); err != nil {
 				return err
diff --git a/operators/pkg/controller/apmserverelasticsearchassociation/apmserverelasticsearchassociation_controller_test.go b/operators/pkg/controller/apmserverelasticsearchassociation/apmserverelasticsearchassociation_controller_test.go
@@ -77,13 +77,7 @@ func Test_deleteOrphanedResources(t *testing.T) {
 					Name:      "as",
 					Namespace: "default",
 				},
-				Spec: apmtype.ApmServerSpec{
-					Output: apmtype.Output{
-						Elasticsearch: apmtype.ElasticsearchOutput{
-							ElasticsearchRef: nil,
-						},
-					},
-				},
+				Spec: apmtype.ApmServerSpec{},
 			},
 			initialObjects: []runtime.Object{
 				&corev1.Secret{
diff --git a/operators/pkg/controller/apmserverelasticsearchassociation/user.go b/operators/pkg/controller/apmserverelasticsearchassociation/user.go
@@ -34,13 +34,18 @@ func apmUserObjectName(assocName string) string {
 
 // userKey is the namespaced name to identify the customer user resource created by the controller.
 func userKey(apm apmtype.ApmServer) *types.NamespacedName {
-
-	ref := apm.Spec.Output.Elasticsearch.ElasticsearchRef
-	if ref == nil {
+	esRef := apm.Spec.ElasticsearchRef
+	if !esRef.IsDefined() {
 		return nil
 	}
+
+	esNamespace := esRef.Namespace
+	if esNamespace == "" {
+		// no namespace given, default to APM's one
+		esNamespace = apm.Namespace
+	}
 	return &types.NamespacedName{
-		Namespace: ref.Namespace,
+		Namespace: esNamespace,
 		Name:      userName(apm),
 	}
 }
@@ -76,7 +81,7 @@ func reconcileEsUser(c k8s.Client, s *runtime.Scheme, apm apmtype.ApmServer, es
 	secretLabels := labels.NewLabels(apm.Name)
 	secretLabels[AssociationLabelName] = apm.Name
 	// add ES labels
-	for k, v := range label.NewLabels(apm.Spec.Output.Elasticsearch.ElasticsearchRef.NamespacedName()) {
+	for k, v := range label.NewLabels(apm.Spec.ElasticsearchRef.NamespacedName()) {
 		secretLabels[k] = v
 	}
 	secKey := secretKey(apm)
@@ -120,7 +125,7 @@ func reconcileEsUser(c k8s.Client, s *runtime.Scheme, apm apmtype.ApmServer, es
 	}
 
 	// analogous to the secret: the user goes on the Elasticsearch side of the association, we apply the ES labels for visibility
-	userLabels := common.NewLabels(apm.Spec.Output.Elasticsearch.ElasticsearchRef.NamespacedName())
+	userLabels := common.NewLabels(apm.Spec.ElasticsearchRef.NamespacedName())
 	userLabels[AssociationLabelName] = apm.Name
 	userLabels[AssociationLabelNamespace] = apm.Namespace
 	expectedEsUser := &corev1.Secret{
diff --git a/operators/pkg/controller/apmserverelasticsearchassociation/user_test.go b/operators/pkg/controller/apmserverelasticsearchassociation/user_test.go
@@ -38,14 +38,11 @@ var apmFixture = apmtype.ApmServer{
 		Namespace: "default",
 	},
 	Spec: apmtype.ApmServerSpec{
-		Output: apmtype.Output{
-			Elasticsearch: apmtype.ElasticsearchOutput{
-				ElasticsearchRef: &commonv1alpha1.ObjectSelector{
-					Name:      "es",
-					Namespace: "default",
-				},
-			},
+		ElasticsearchRef: commonv1alpha1.ObjectSelector{
+			Name:      "es",
+			Namespace: "default",
 		},
+		Elasticsearch: apmtype.ElasticsearchOutput{},
 	},
 }
 
@@ -186,14 +183,11 @@ func Test_reconcileEsUser(t *testing.T) {
 						Namespace: "ns-2",
 					},
 					Spec: apmtype.ApmServerSpec{
-						Output: apmtype.Output{
-							Elasticsearch: apmtype.ElasticsearchOutput{
-								ElasticsearchRef: &commonv1alpha1.ObjectSelector{
-									Name:      "es",
-									Namespace: "ns-1",
-								},
-							},
+						ElasticsearchRef: commonv1alpha1.ObjectSelector{
+							Name:      "es",
+							Namespace: "ns-1",
 						},
+						Elasticsearch: apmtype.ElasticsearchOutput{},
 					},
 				},
 			},
diff --git a/operators/pkg/controller/common/association/association_test.go b/operators/pkg/controller/common/association/association_test.go
diff --git a/operators/test/e2e/apm/standalone_test.go b/operators/test/e2e/apm/standalone_test.go
diff --git a/operators/test/e2e/test/apmserver/builder.go b/operators/test/e2e/test/apmserver/builder.go

Original file line number	Diff line number	Diff line change
`@@ -309,7 +309,7 @@ func (r *ReconcileApmServer) deploymentParams(`
`309`	`309`	`_, _ = configChecksum.Write([]byte(params.keystoreResources.Version))`
`310`	`310`	`}`
`311`	`311`
`312`		`- esCASecretName := as.Spec.Output.Elasticsearch.SSL.CertificateAuthorities.SecretName`
	`312`	`+ esCASecretName := as.Spec.Elasticsearch.SSL.CertificateAuthorities.SecretName`
`313`	`313`	`if esCASecretName != "" {`
`314`	`314`	`// TODO: use apmServerCa to generate cert for deployment`
`315`	`315`