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

Simplify setup of Enterprise Search access through Kibana UI #4598

Merged
merged 29 commits into from Jul 5, 2021

Conversation

sebgl
Copy link
Contributor

@sebgl sebgl commented Jun 29, 2021

Starting version 7.14.0, Kibana becomes the default UI for Enterprise Search.
This PR introduces an enterpriseSearchRef in the Kibana CRD, to simplify the Kibana -> Enterprise Search connection configuration.

Sample manifests to try it out:

apiVersion: elasticsearch.k8s.elastic.co/v1
kind: Elasticsearch
metadata:
  name: es-test
spec:
  version: 7.14.0
  image: docker.elastic.co/elasticsearch/elasticsearch:7.14.0-SNAPSHOT
  nodeSets:
    - name: default
      count: 1
      config:
        node.store.allow_mmap: false
---
apiVersion: enterprisesearch.k8s.elastic.co/v1beta1
kind: EnterpriseSearch
metadata:
  name: entsearch-test
spec:
  version: 7.14.0
  image: docker.elastic.co/enterprise-search/enterprise-search:7.14.0-SNAPSHOT
  count: 1
  elasticsearchRef:
    name: es-test
---
apiVersion: kibana.k8s.elastic.co/v1
kind: Kibana
metadata:
  name: kibana-test
spec:
  version: 7.14.0
  image: docker.elastic.co/kibana/kibana:7.14.0-SNAPSHOT
  count: 1
  elasticsearchRef:
    name: es-test
  enterpriseSearchRef:
    name: entsearch-test

You can then access Enterprise Search by clicking the "Enterprise Search" button in Kibana UI.


Some technical details:

This is the first time we introduce an association between two resources that does not involve creating an Elasticsearch user along the way. Therefore, aside from the "simple" additional Kibana -> Enterprise Search association, this PR refactors the association controller to allow associations with no transitive Elasticsearch resource, and no additional user creation.

A few noticeable changes:

  • Each association's AssociationInfo now contains an optional ElasticsearchUserCreation section to embed the Elasticsearch user creation details (nil in the Kibana -> Enterprise Search association).
  • The existing association conf annotation remains unmodified, however a special value can be set on the auth secret name to indicate no auth configuration is required: authSecretName: "-" (as opposed to "" which means the value has not been set yet).
  • We now explicitly check whether the referenced resource (not always Elasticsearch!) exists at the very beginning of the association reconciliation.
  • I refactored the dynamic watches to ensure we always watch the referenced resource (can be done in a generic way), and conditionally watch the es user secret. No need for SetDynamicWatches in the AssociationInfo anymore.

The PR also adds an integration test to cover the Kibana -> Enterprise Search association, and checks whether the Enterprise Search API is correctly available through Kibana.

Remaining to be done:

  • Documentation (through another PR)

@sebgl sebgl added >feature Adds or discusses adding a feature to the product v1.7.0 labels Jun 29, 2021

// NoAuthRequiredValue is the value set for AuthSecretName if no authentication
// credentials are necessary for that association.
NoAuthRequiredValue = "-"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm assuming here that no secret will ever be named "-". Since this is a value we set with our own secret naming scheme it should be OK.
It's somewhat nice because it doesn't break the existing schema and is quite easy to manipulate in the code.
However it also feels a bit "implicit", we could also modify the assoc conf schema to make things more explicit. WDYT?

@sebgl
Copy link
Contributor Author

sebgl commented Jun 30, 2021

Jenkins test this please

Copy link
Collaborator

@pebrc pebrc left a comment

Choose a reason for hiding this comment

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

Looks good! I like the association refactoring beyond adding the required capabilities I think it is an improvement in general.

We now explicitly check whether the referenced resource (not always Elasticsearch!) exists at the very beginning of the association reconciliation. An additional ReferencedResourceExists() function needs to be implemented by each association controller that knows about the referenced object type.

I think you gave up on that idea and found a generic way with k8s.ObjectExists

pkg/apis/common/v1/association_test.go Outdated Show resolved Hide resolved
pkg/apis/kibana/v1/kibana_types.go Outdated Show resolved Hide resolved
pkg/controller/association/controller/apm_kibana.go Outdated Show resolved Hide resolved
pkg/controller/association/controller/kibana_ent.go Outdated Show resolved Hide resolved
pkg/controller/association/reconciler_test.go Show resolved Hide resolved
pkg/controller/association/reconciler_test.go Show resolved Hide resolved
Copy link
Collaborator

@pebrc pebrc left a comment

Choose a reason for hiding this comment

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

LGTM but I think we need to regenerate the API docs.

@sebgl
Copy link
Contributor Author

sebgl commented Jul 5, 2021

Jenkins test this please

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>feature Adds or discusses adding a feature to the product v1.7.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants