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

Deprecate username/password in elastic-agent #29434

Merged

Conversation

michel-laterman
Copy link
Contributor

@michel-laterman michel-laterman commented Dec 15, 2021

What does this PR do?

Add deprecation logs when username/password is detected by the elastic-agent.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Related issues

@michel-laterman michel-laterman added release-note:deprecation The content should be included in the deprecation section v7.16.0 Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team labels Dec 15, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/elastic-agent-control-plane (Team:Elastic-Agent-Control-Plane)

@botelastic botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Dec 15, 2021
@@ -5,8 +5,11 @@ outputs:
default:
type: elasticsearch
hosts: [127.0.0.1:9200]
username: elastic
password: changeme
api_key: "example-key"
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 a bit confused if this attribute should be api_key or service_token.
the libbeat output config expects api_key, but some config from within the agent refers to it as service_token.

Copy link
Member

Choose a reason for hiding this comment

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

api_key is for the output, service_token is for what is passed to fleet-server. This is not the same thing, see other comments.

@elasticmachine
Copy link
Collaborator

elasticmachine commented Dec 15, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2021-12-21T17:16:47.761+0000

  • Duration: 102 min 43 sec

  • Commit: d18269e

Test stats 🧪

Test Results
Failed 0
Passed 7069
Skipped 24
Total 7093

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /package : Generate the packages and run the E2E tests.

  • /beats-tester : Run the installation tests with beats-tester.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

Copy link
Contributor

@michalpristas michalpristas left a comment

Choose a reason for hiding this comment

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

could you describe api_key and service_token and their usage?
i see some outputs gained token other ones api_key

@ruflin ruflin changed the base branch from 7.16 to 7.17 December 16, 2021 10:22
@ruflin
Copy link
Member

ruflin commented Dec 16, 2021

I retargeted this PR to 7.17. If we need it also in 7.16, lets make sure it is backported.

@@ -150,3 +150,4 @@
- Add diagnostics collect command to gather beat metadata, config, policy, and logs and bundle it into an archive. {pull}28461[28461]
- Add `KIBANA_FLEET_SERVICE_TOKEN` to Elastic Agent container. {pull}28096[28096]
- Allow pprof endpoints for elastic-agent or beats if enabled. {pull}28983[28983] {pull}29155[29155]
- Mark username/password settings as deprecated. {pull}29434[29434]
Copy link
Member

Choose a reason for hiding this comment

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

username / password for fleet-server are deprecated. If we should deprecate username / password for Elastic Agent in a more general way for the output is another discussion.

username: elastic
password: changeme
api_key: "example-key"
# Note that basic auth is deprecated and will be removed in 8.0
Copy link
Member

Choose a reason for hiding this comment

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

This does not apply to the general output. +1 on having the api_key here but username / password for the output part stay around AFAIK.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah, much simpler then. i'll just log it there.

@@ -5,8 +5,11 @@ outputs:
default:
type: elasticsearch
hosts: [127.0.0.1:9200]
username: elastic
password: changeme
api_key: "example-key"
Copy link
Member

Choose a reason for hiding this comment

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

api_key is for the output, service_token is for what is passed to fleet-server. This is not the same thing, see other comments.

@michel-laterman
Copy link
Contributor Author

/test

Copy link
Member

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

Lets make sure we also follow up with docs to deprecate it also in the docs.

Copy link
Contributor

@michalpristas michalpristas left a comment

Choose a reason for hiding this comment

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

also please as a followup coordinate with cloud/or make sure they're aware of the upcoming change

Copy link
Member

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

I think this also needs an update on

FLEET_SERVER_ELASTICSEARCH_USERNAME - elasticsearch username for Fleet Server [$ELASTICSEARCH_USERNAME]
Add a note that these are deprecated as this is what shows up in the help output.

@michel-laterman
Copy link
Contributor Author

/test

Copy link
Member

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

Change LGTM. Lets try to get CI pass and get it in.

@michel-laterman michel-laterman merged commit 1810b78 into elastic:7.17 Dec 23, 2021
@michel-laterman michel-laterman deleted the agent-deprecate-password branch December 23, 2021 17:22
mergify bot pushed a commit that referenced this pull request Dec 23, 2021
* Deprecate username/password in elastic-agent

* Appy deprecation only to fleet-server

* Review feedback

* Add deprecation notes in container help output

(cherry picked from commit 1810b78)
michel-laterman added a commit that referenced this pull request Dec 23, 2021
* Deprecate username/password in elastic-agent

* Appy deprecation only to fleet-server

* Review feedback

* Add deprecation notes in container help output

(cherry picked from commit 1810b78)

Co-authored-by: Michel Laterman <82832767+michel-laterman@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-v7.16.0 Automated backport with mergify release-note:deprecation The content should be included in the deprecation section Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team v7.16.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants