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

APM: remove "output" element and add elasticsearchRef #1345

Merged
merged 5 commits into from
Jul 23, 2019
Merged
Show file tree
Hide file tree
Changes from 2 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
113 changes: 54 additions & 59 deletions operators/config/crds/apm_v1alpha1_apmserver.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
7 changes: 2 additions & 5 deletions operators/config/samples/apm/apm_es_kibana.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 7 additions & 11 deletions operators/pkg/apis/apm/v1alpha1/apmserver_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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"`
Expand Down Expand Up @@ -148,7 +144,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 {
Expand Down
25 changes: 2 additions & 23 deletions operators/pkg/apis/apm/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion operators/pkg/controller/apmserver/apmserver_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,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

Expand Down
4 changes: 2 additions & 2 deletions operators/pkg/controller/apmserver/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,13 +204,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
Expand Down Expand Up @@ -240,8 +243,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(
Expand All @@ -257,8 +258,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
Expand All @@ -285,7 +286,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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
16 changes: 11 additions & 5 deletions operators/pkg/controller/apmserverelasticsearchassociation/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,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 {
ref := apm.Spec.ElasticsearchRef
if !ref.IsDefined() {
return nil
}

esNamespace := apm.Spec.ElasticsearchRef.Namespace
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: we could reuse the ref variable: esNamespace := ref.Namespace. I may be a bit rigid but I would appreciate to have the same block of code with the same namings in the 2 controllers.

esRef := as.Spec.Output.Elasticsearch.ElasticsearchRef
if esRef == nil {
	return nil
}

if esRef.Namespace == "" {
	// no namespace provided: default to the APM Server namespace
	esRef.Namespace = as.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),
}
}
Expand Down Expand Up @@ -76,7 +82,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)
Expand Down Expand Up @@ -120,7 +126,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{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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{},
},
}

Expand Down Expand Up @@ -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{},
},
},
},
Expand Down
Loading