Feature/4890 detect fail early upgrade #5864
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This pull request does not have a backport label. Could you fix it @kaanyalti? 🙏
|
|
|
mergify
bot
added
the
backport-8.x
pierrehilbert
added
the
Team:Elastic-Agent-Control-Plane
|
Pinging @elastic/elastic-agent-control-plane (Team:Elastic-Agent-Control-Plane) |
kaanyalti
force-pushed
the
feature/4890_detect_fail_early_upgrade
branch
from
b8ea9bb to
b3c85b0
Compare
changelog/fragments/1729971565-detect-fail-early-fleet-managed-cli-upgrade.yaml
Outdated
Show resolved
Hide resolved
pchila
reviewed
Oct 28, 2024
kaanyalti
force-pushed
the
feature/4890_detect_fail_early_upgrade
branch
from
bcabfba to
022c342
Compare
kaanyalti
force-pushed
the
feature/4890_detect_fail_early_upgrade
branch
from
69a78a0 to
f6ed834
Compare
kaanyalti
commented
Oct 29, 2024
pchila
reviewed
Oct 30, 2024
kaanyalti
force-pushed
the
feature/4890_detect_fail_early_upgrade
branch
from
8a76882 to
621c3fb
Compare
andrzej-stencel
approved these changes
Oct 31, 2024
cmacknz
reviewed
Nov 1, 2024
|
I tried to manually test that upgrading as a non-root user fails, I did the following:
sudo ./elastic-agent install -f --unprivileged --develop
Unprivileged installation mode enabled; this feature is currently in beta.
Installing into development namespace; this is an experimental and currently unsupported feature.
[=== ] Service Started [6s] Elastic Agent successfully installed, starting enrollment.
[=== ] Done [6s]
Elastic Agent has been successfully installed.
sudo -u elastic-agent-user elastic-development-agent version
Binary: 1.0.0-SNAPSHOT (build: 269031487ee30d3ec60058c1743e01eedd21dd2b at 2024-11-01 18:23:57 +0000 UTC)
Daemon: 1.0.0-SNAPSHOT (build: 269031487ee30d3ec60058c1743e01eedd21dd2b at 2024-11-01 18:23:57 +0000 UTC)
I was expecting to be stopped, but maybe this is a bug in how we detect we are root when doing |
|
OK, if I enroll in Fleet it works I see what has happened: ❯ sudo -u elastic-agent-user elastic-development-agent upgrade 8.15.3
Error: aborting upgrade: upgrade command needs to be executed as root for fleet managed agents
For help, please see our troubleshooting guide at https://www.elastic.co/guide/en/fleet/9.0/fleet-troubleshooting.html |
|
We only require being root to use the CLI with --force if enrolled with Fleet, the standalone check comes first. We could require being root to upgrade an unprivileged agent, it makes sense to me as a system management action, but it would be a breaking change for it today. It does appear that the escape hatch for upgrades doesn't work on a Mac though for unprivileged agents: ❯ sudo -u elastic-agent-user elastic-development-agent upgrade 8.15.3 --force
Error: aborting upgrade: upgrade command needs to be executed as root for fleet managed agents
For help, please see our troubleshooting guide at https://www.elastic.co/guide/en/fleet/9.0/fleet-troubleshooting.html
~/Downloads/elastic-agent-9.0.0-SNAPSHOT-darwin-aarch64 ·································· 02:58:47 PM
❯ sudo elastic-development-agent upgrade 8.15.3 --force
Error: failed to retrieve agent info while tring to upgrade the agent: could not get agent info from store: fail to read configuration /Library/Elastic/Agent-Development/fleet.enc for the agent: fail to decode bytes: cipher: message authentication failed
For help, please see our troubleshooting guide at https://www.elastic.co/guide/en/fleet/9.0/fleet-troubleshooting.htmlI can't upgrade as root or as Edit: I'm using 9.0.0 know but it's the same commit from this PR being tested: sudo elastic-development-agent version
Binary: 9.0.0-SNAPSHOT (build: 269031487ee30d3ec60058c1743e01eedd21dd2b at 2024-11-01 18:11:36 +0000 UTC)
Daemon: 9.0.0-SNAPSHOT (build: 269031487ee30d3ec60058c1743e01eedd21dd2b at 2024-11-01 18:11:36 +0000 UTC) |
Looking into this right away |
Co-authored-by: Paolo Chilà <paolo.chila@elastic.co>
Co-authored-by: Paolo Chilà <paolo.chila@elastic.co>
Co-authored-by: Craig MacKenzie <craig.mackenzie@elastic.co>
- updated agentinfo proto - updated relevant generated code - implemented state call in the upgrade cmd
Co-authored-by: Andrzej Stencel <andrzej.stencel@elastic.co>
kaanyalti
force-pushed
the
feature/4890_detect_fail_early_upgrade
branch
from
33f9e17 to
3de964e
Compare
|
mergify bot
pushed a commit
that referenced
this pull request
Nov 8, 2024
* feature(4890): added shouldUpgrade function in the upgrade cli file * feature(4890): added shouldUpgrade check into the upgrade command * feature(4890): ran gofmt * feature(4890): added a "force" flag, marked it as hidden * feature(4890): removed dpkg, rpm and container logic * feature(4890): ran gofmt * feature(4890): updated the function signature of the upgrade command, updated tests, added new tests * feature(4890): update comments * feature(4890): added changelog fragment * feature(4890): added fatal log in case there is an error while marking force flag as hidden * feature(4890): added error checks in tests * feature(4890): updated the summary in the changelog fragment * feature(4890): removed the shorthand flag for the force flag * feature(4890): updated synchronization in the tests * Update internal/pkg/agent/cmd/upgrade_test.go Co-authored-by: Paolo Chilà <paolo.chila@elastic.co> * feature(4890): using streams err output instead of defaulting to stderr * feature(4890): use EXPECT instead of On * feature(4890): moved unconfirmed upgrade error to a package var * feature(4890): removed confirmation from upgrade check for when force flag is set * Update internal/pkg/agent/cmd/upgrade.go Co-authored-by: Paolo Chilà <paolo.chila@elastic.co> * Update internal/pkg/agent/cmd/upgrade.go Co-authored-by: Paolo Chilà <paolo.chila@elastic.co> * feature(4890): fix errors * Update internal/pkg/agent/cmd/upgrade.go Co-authored-by: Craig MacKenzie <craig.mackenzie@elastic.co> * feature(4890): update test * fearure(4890): replace ageninfo with state call - updated agentinfo proto - updated relevant generated code - implemented state call in the upgrade cmd * feature(4890): updated proto, client and server implementation * feature(4890): fix struct tag * feature(4890): added skip-verify checks * feature(4890): ran addLicenseHeaders * feature(4890): ran mage clean * feature(4890): fix typo * feature(4890): added timeout to connection * feature(4890): changed condition check order * feature(4890): fix unit tests * feature(4890): refactored tests, using mock client * Update internal/pkg/agent/cmd/upgrade.go Co-authored-by: Andrzej Stencel <andrzej.stencel@elastic.co> * feature(4890): use lower case "f" in error messages to be more consistent * feature(4890): remove duplicate line * feature(4890): ran mage controlProto with correct protoc version --------- Co-authored-by: Paolo Chilà <paolo.chila@elastic.co> Co-authored-by: Craig MacKenzie <craig.mackenzie@elastic.co> Co-authored-by: Andrzej Stencel <andrzej.stencel@elastic.co> (cherry picked from commit 8579474)
4 tasks
kaanyalti
added a commit
that referenced
this pull request
Nov 13, 2024
* feature(4890): added shouldUpgrade function in the upgrade cli file * feature(4890): added shouldUpgrade check into the upgrade command * feature(4890): ran gofmt * feature(4890): added a "force" flag, marked it as hidden * feature(4890): removed dpkg, rpm and container logic * feature(4890): ran gofmt * feature(4890): updated the function signature of the upgrade command, updated tests, added new tests * feature(4890): update comments * feature(4890): added changelog fragment * feature(4890): added fatal log in case there is an error while marking force flag as hidden * feature(4890): added error checks in tests * feature(4890): updated the summary in the changelog fragment * feature(4890): removed the shorthand flag for the force flag * feature(4890): updated synchronization in the tests * Update internal/pkg/agent/cmd/upgrade_test.go Co-authored-by: Paolo Chilà <paolo.chila@elastic.co> * feature(4890): using streams err output instead of defaulting to stderr * feature(4890): use EXPECT instead of On * feature(4890): moved unconfirmed upgrade error to a package var * feature(4890): removed confirmation from upgrade check for when force flag is set * Update internal/pkg/agent/cmd/upgrade.go Co-authored-by: Paolo Chilà <paolo.chila@elastic.co> * Update internal/pkg/agent/cmd/upgrade.go Co-authored-by: Paolo Chilà <paolo.chila@elastic.co> * feature(4890): fix errors * Update internal/pkg/agent/cmd/upgrade.go Co-authored-by: Craig MacKenzie <craig.mackenzie@elastic.co> * feature(4890): update test * fearure(4890): replace ageninfo with state call - updated agentinfo proto - updated relevant generated code - implemented state call in the upgrade cmd * feature(4890): updated proto, client and server implementation * feature(4890): fix struct tag * feature(4890): added skip-verify checks * feature(4890): ran addLicenseHeaders * feature(4890): ran mage clean * feature(4890): fix typo * feature(4890): added timeout to connection * feature(4890): changed condition check order * feature(4890): fix unit tests * feature(4890): refactored tests, using mock client * Update internal/pkg/agent/cmd/upgrade.go Co-authored-by: Andrzej Stencel <andrzej.stencel@elastic.co> * feature(4890): use lower case "f" in error messages to be more consistent * feature(4890): remove duplicate line * feature(4890): ran mage controlProto with correct protoc version --------- Co-authored-by: Paolo Chilà <paolo.chila@elastic.co> Co-authored-by: Craig MacKenzie <craig.mackenzie@elastic.co> Co-authored-by: Andrzej Stencel <andrzej.stencel@elastic.co> (cherry picked from commit 8579474) Co-authored-by: Kaan Yalti <kaan.yalti@elastic.co>
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
This PR restricts using the cli to upgrade fleet managed elastic agents. With this implementation, the only way to upgrade a fleet managed agent using the cli is only if the agent is running in privileged mode and the user provides a
forceflag.Checklist
[ ] I have made corresponding changes to the documentation[ ] I have made corresponding change to the default configuration files./changelog/fragmentsusing the changelog tool[ ] I have added an integration test or an E2E testDisruptive User Impact
If users are upgrading fleet-managed agents using the cli for some reason, they should be aware that the upgrade command will stop working for agents running in unprivileged mode. For any fleet managed agents running in privileged mode, users will need to go through an interactive step and use the
--forceflag to be able to upgrade an agent.How to test this PR locally
Setup
DEV=true SNAPSHOT=true mage build--developflag to your installation commandTest cases
Unprivileged
--unprivilegedflag to your installation commandsudo -u elastic-agent-user elastic-development-agent upgrade <VERSION>. The command should not execute.--forceflag does not change the behaviorPrivileged
sudo elastic-development-agent upgrade <VERSION>. The command should not execute--forceflagsudo elastcit-development-agent upgrade <VERSION> --force.Related issues