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.

Sign up for GitHub

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

Jump to bottom

Introduce a restricted role for APM agent configuration #3155

Merged
merged 3 commits into from
Jun 2, 2020
Merged

Introduce a restricted role for APM agent configuration #3155

merged 3 commits into from
Jun 2, 2020

Conversation

barkbay
Copy link
Contributor

@barkbay barkbay commented May 28, 2020 •
edited
Loading

While doing some manual testing with the APM Agent configuration I realized that the kibana_user role used in #3043 does not work for 7.3 and 7.4

This PR:

  • Set 7.5.1 as the minimum version required to use agent configuration. This feature is GA since 7.5 and supporting older versions would require to setup some dedicated roles.
  • Introduce a restricted role to be used by the agent configuration user:
eck_apm_agent_user_role:
  cluster: []
  indices: []
  applications:
  - application: kibana-.kibana
    privileges:
    - feature_apm.read
    resources:
    - space:default

This PR has been tested with 7.5.2 , 7.6.2 and 7.7.0

@barkbay barkbay added >enhancement Enhancement of existing functionality v1.2.0 labels May 28, 2020
barkbay
barkbay commented May 28, 2020
View reviewed changes
pkg/controller/elasticsearch/user/roles.go
ApmAgentUserRole: esclient.Role{
Cluster: []string{},
Indices: []esclient.IndexRole{},
Applications: []esclient.ApplicationRole{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@simitt @sqren would be glad to have your review on this role. Thanks 🙏

Copy link
Member

@sorenlouv sorenlouv May 28, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

lgtm 👍
fyi: In parallel I'm working on improving this by making this behaviour default for the apm_user role: elastic/elasticsearch#57201.

It might not be available until 8.0 (so still quite a long way out) but better than never.

simitt
simitt approved these changes May 28, 2020
View reviewed changes
Copy link

@simitt simitt 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 let's also wait for feedback from @sqren

@barkbay
Copy link
Contributor Author

barkbay commented Jun 2, 2020

While working on the e2e tests I noticed that something seems to be broken in 7.5.0 While I had no issues with 7.5.1 and 7.5.2. (could be related to elastic/apm-server#3031 ?)

@barkbay barkbay merged commit 57269ec into elastic:master Jun 2, 2020
anyasabo
anyasabo reviewed Jun 2, 2020
View reviewed changes
pkg/apis/apm/v1/webhook_test.go
apm.Spec.KibanaRef = commonv1.ObjectSelector{Name: "kbname", Namespace: "kbns"}
return serialize(t, apm)
},
Check: test.ValidationWebhookFailed(
`spec.kibanaRef: Forbidden: required version for Kibana association is 7.3.0 but desired version is 7.2.0`,
`spec.kibanaRef: Forbidden: required version for Kibana association is 7.5.1 but desired version is 7.4.0`,
Copy link
Contributor

Choose a reason for hiding this comment

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

Late nit: should read "minimum required version..."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback, I'll fix this as part of #3154

@anyasabo
Copy link
Contributor

anyasabo commented Jun 2, 2020

Late comment: I wonder if we want to update our compatibility docs. I think we might not have to since it was only GA in 7.5

@charith-elastic charith-elastic changed the title Introduce a restricted role for agent configuration Introduce a restricted role for APM agent configuration Jun 25, 2020
@barkbay barkbay mentioned this pull request Jul 1, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anyasabo anyasabo anyasabo left review comments

@simitt simitt simitt approved these changes

@sebgl sebgl sebgl approved these changes

@sorenlouv sorenlouv sorenlouv approved these changes

Assignees
No one assigned
Labels
>enhancement Enhancement of existing functionality v1.2.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@barkbay @anyasabo @sorenlouv @sebgl @simitt