Skip to content

[Elastic Agent] Fix issues with enrollment key fetching#24319

Merged
blakerouse merged 3 commits intoelastic:masterfrom
blakerouse:fix-24310
Mar 4, 2021
Merged

[Elastic Agent] Fix issues with enrollment key fetching#24319
blakerouse merged 3 commits intoelastic:masterfrom
blakerouse:fix-24310

Conversation

@blakerouse
Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes an issue with the new container sub-command. It was not handling the naming of the enrollment tokens correctly and when not running without Fleet Server the api_key is not returned in the listing, requiring a detailed fetch to retrieve it.

Why is it important?

So usage of the Elastic Agent container on 7.13 works.

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

@blakerouse blakerouse added the Team:Fleet Label for the Fleet team label Mar 3, 2021
@blakerouse blakerouse self-assigned this Mar 3, 2021
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/fleet (Team:Fleet)

@botelastic botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label Team:Ingest Management labels Mar 3, 2021
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/ingest-management (Team:Ingest Management)

@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Mar 3, 2021
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Mar 3, 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

Expand to view the summary

Build stats

  • Build Cause: Pull request #24319 updated

  • Start Time: 2021-03-04T15:11:37.760+0000

  • Duration: 50 min 21 sec

  • Commit: f1bd2a3

Test stats 🧪

Test Results
Failed 0
Passed 6548
Skipped 16
Total 6564

Trends 🧪

Image of Build Times

Image of Tests

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test Results
Failed 0
Passed 6548
Skipped 16
Total 6564

@andresrc andresrc added the Team:Elastic-Agent Label for the Agent team label Mar 4, 2021
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/agent (Team:Agent)

Copy link
Copy Markdown
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.

LGTM

)

var (
tokenNameStrip = regexp.MustCompile(`\s\([0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}\)$`)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can you add a comment here why and what this is?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

added

tokenName := envWithDefault(defaultTokenName, "FLEET_TOKEN_NAME")
for _, key := range keys {
name := strings.TrimSpace(strings.Replace(key.Name, fmt.Sprintf(" (%s)", key.ID), "", 1))
name := tokenNameStrip.ReplaceAllString(key.Name, "")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can it be that there will be trailing/leading space after replace as these were removed before

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

nice catch, I added the strings.TrimSpace back in

@mtojek
Copy link
Copy Markdown
Contributor

mtojek commented Mar 4, 2021

Please remember about backporting this issue, so we can pull it to the 7.13.0-SNAPSHOT.

@blakerouse blakerouse merged commit 3f00537 into elastic:master Mar 4, 2021
@blakerouse blakerouse deleted the fix-24310 branch March 4, 2021 22:19
blakerouse added a commit to blakerouse/beats that referenced this pull request Mar 4, 2021
* Fix issues with enrollment key fetching for Kibana 7.13.

* Fix issue with regex.

* Add comment, add strings.TrimSpace

(cherry picked from commit 3f00537)
blakerouse added a commit that referenced this pull request Mar 5, 2021
)

* Fix issues with enrollment key fetching for Kibana 7.13.

* Fix issue with regex.

* Add comment, add strings.TrimSpace

(cherry picked from commit 3f00537)
@ruflin
Copy link
Copy Markdown
Contributor

ruflin commented Mar 5, 2021

If there are multiple keys with the same name, which one will it take?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Team:Elastic-Agent Label for the Agent team Team:Fleet Label for the Fleet team v7.13.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[7.13.0-SNAPSHOT] Elastic-Agent: unable to find enrollment token named "Default" in policy "Default policy"

7 participants