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

Enable custom roles and spaces in serverless projects #195584

Merged
merged 28 commits into from
Oct 21, 2024

Conversation

jeramysoucy
Copy link
Contributor

@jeramysoucy jeramysoucy commented Oct 9, 2024

Closes #194933
Closes #192282

Summary

This PR updates the serverless project yml files to

  • enable custom roles for Elasticsearch and Security projects
  • enable multiple spaces (max 100) for all serverless project types

Tests

Additionally, this PR adjust the serverless test suites. Originally, testing of roles and spaces endpoints was achieved from the feature flag test config. Now that these features are enabled by default, the tests have been migrated to the standard serverless test configs.

Affected tests:

  • x-pack/test_serverless/api_integration/test_suites/common/management/spaces.ts
  • x-pack/test_serverless/api_integration/test_suites/common/platform_security/authorization.ts
  • x-pack/test_serverless/functional/test_suites/common/platform_security/navigation/management_nav_cards.ts
  • x-pack/test_serverless/functional/test_suites/common/platform_security/roles.ts
  • x-pack/test_serverless/functional/test_suites/common/spaces/spaces_management.ts
  • x-pack/test_serverless/functional/test_suites/common/spaces/spaces_selection.ts
  • Feature flag configs/indices
  • Project specific configs/indices
  • Base serverless config

@jeramysoucy jeramysoucy changed the title Enables custom roles and spaces in serverless projects Enable custom roles and spaces in serverless projects Oct 9, 2024
@jeramysoucy jeramysoucy added the Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! label Oct 9, 2024
@jeramysoucy jeramysoucy self-assigned this Oct 9, 2024
@jeramysoucy jeramysoucy added v9.0.0 v8.16.0 release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting labels Oct 9, 2024
@@ -22,13 +22,12 @@ export default createTestConfig({
// add feature flags
kbnServerArgs: [
'--xpack.infra.enabled=true',
'--xpack.security.roleManagementEnabled=true', // enables custom roles
`--xpack.spaces.maxSpaces=100`, // enables spaces UI capabilities
'--xpack.security.roleManagementEnabled=true', // needed to check composite feautures in /observability/platform_security/authorization.ts
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@azasypkin Should we expose the api/security/privileges endpoint in serverless even if role management is off? Currently, we do not.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I’d say no, we shouldn’t. But would you mind giving me a bit of context? Do I understand correctly that we’ll still keep role/privilege-related tests for O11y, but only via the “feature_flags” config (unlike other project types for which we can finally move to base/normal configs)?

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, I just wanted to confirm.

would you mind giving me a bit of context? Do I understand correctly that we’ll still keep role/privilege-related tests for O11y, but only via the “feature_flags” config (unlike other project types for which we can finally move to base/normal configs)?

Pretty much, yes. Specifics: there is a test to confirm the "composite features" functionality here:

This test calls the api/security/privileges endpoint to validate how the oblt features have been configured for serverless, but this endpoint is only available when we enable role management so it needs to remain in the feature flag config until either oblt enabled custom roles, or we decide to register this route for other reasons.

Copy link
Member

Choose a reason for hiding this comment

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

but this endpoint is only available when we enable role management so it needs to remain in the feature flag config until either oblt enabled custom roles

Ah, got it, thanks, it makes sense to me 👍

config/serverless.es.yml Outdated Show resolved Hide resolved
@@ -22,13 +22,12 @@ export default createTestConfig({
// add feature flags
kbnServerArgs: [
'--xpack.infra.enabled=true',
'--xpack.security.roleManagementEnabled=true', // enables custom roles
`--xpack.spaces.maxSpaces=100`, // enables spaces UI capabilities
'--xpack.security.roleManagementEnabled=true', // needed to check composite feautures in /observability/platform_security/authorization.ts
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I’d say no, we shouldn’t. But would you mind giving me a bit of context? Do I understand correctly that we’ll still keep role/privilege-related tests for O11y, but only via the “feature_flags” config (unlike other project types for which we can finally move to base/normal configs)?

@jeramysoucy jeramysoucy marked this pull request as ready for review October 11, 2024 13:17
@jeramysoucy jeramysoucy requested review from a team as code owners October 11, 2024 13:17
Copy link
Contributor

@jloleysens jloleysens left a comment

Choose a reason for hiding this comment

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

Nice work @jeramysoucy !

Copy link
Contributor

@eokoneyo eokoneyo left a comment

Choose a reason for hiding this comment

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

Shared UX changes LGTM

Copy link
Member

@dmlemeshko dmlemeshko left a comment

Choose a reason for hiding this comment

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

appex-qa changes LGTM

Comment on lines 170 to 178
// normally below is injected by control plane
`--xpack.cloud.serverless.project_id=fakeprojectid`,
`--xpack.cloud.base_url=https://fake-cloud.elastic.co`,
`--xpack.cloud.projects_url=/projects/`,
`--xpack.cloud.profile_url=/user/settings/`,
`--xpack.cloud.billing_url=/billing/overview/`,
`--xpack.cloud.deployments_url=/deployments`,
`--xpack.cloud.organization_url=/account/`,
`--xpack.cloud.users_and_roles_url=/account/members/`,
Copy link
Member

Choose a reason for hiding this comment

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

Thanks a lot for addressing it!

Copy link
Contributor

@yngrdyn yngrdyn left a comment

Choose a reason for hiding this comment

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

Test changes LGTM

@azasypkin azasypkin self-requested a review October 17, 2024 09:38
@azasypkin
Copy link
Member

ACK: will review today

Copy link
Member

@azasypkin azasypkin left a comment

Choose a reason for hiding this comment

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

Great job! I left a few nits and one question regarding running custom roles tests for O11y once the team is ready to develop and test them.

config/serverless.yml Outdated Show resolved Hide resolved
x-pack/test_serverless/shared/config.base.ts Show resolved Hide resolved
],
// load tests in the index file
testFiles: [require.resolve('./index.feature_flags.ts')],

// include settings from project controller
// https://github.com/elastic/project-controller/blob/main/internal/project/observability/config/elasticsearch.yml
esServerArgs: ['xpack.ml.dfa.enabled=false', 'xpack.security.authc.native_roles.enabled=true'],
esServerArgs: ['xpack.ml.dfa.enabled=false'],
Copy link
Member

Choose a reason for hiding this comment

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

question: When the O11y solution is ready to start working on custom roles functionality and would like to run tests related to roles regularly - locally, on CI, and in MKI - but aren’t ready to enable roles in production yet, what would be our recommendation? Will they need to remove "skip" flags and add the necessary overrides in GitOps to enable custom roles functionality in QA only to ensure we can run MKI tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question. I guess we have a couple of options:

  1. Retain the feature flag tests for OBLT to use (however, I don't think these get run on MKI)
  2. Adjust skip flags and FTR config, enable in QA/Staging, keep disabled in prod

Maybe @pheyos has a better suggestion?

Copy link
Member

@pheyos pheyos Oct 21, 2024

Choose a reason for hiding this comment

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

I think we only rarely want to enable feature flags broadly in a whole env for testing and custom roles for Observability does not really qualify for an exception there IMO. Here's what I think they should do once they're ready to test:

  • get feature flag tests in - doesn't run on MKI but shows that things are functional
  • create one or more test projects in MKI and override the setting for these projects in order to test end-to-end
    • run the feature flag tests against this project

If that's all looking good:

  • make sure no existing tests are breaking if custom roles are enabled
  • step by step enabled the feature in QA, Staging and Production
  • then move the feature flag tests to regular tests

Copy link
Contributor Author

@jeramysoucy jeramysoucy Oct 21, 2024

Choose a reason for hiding this comment

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

Thanks, Robert! @azasypkin Do you think we should we split out the roles tests and set the feature flag test back up for OBLT now? Not sure what their timeframe is for enabling custom role; it sounds like it is not in their roadmap - I'll forward you an email I got from Chris DiStasio.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, @jeramysoucy. TL;DR: let’s keep what you have here.

Since there are no immediate signs that O11y is planning to introduce custom roles anytime soon (though I believe this might change quickly with GA), I don’t think we need to make the testing setup more complex than it needs to be. Once the O11y team is ready, we’ll propose what Robert outlined above and provide any necessary assistance if needed.

@jeramysoucy
Copy link
Contributor Author

Thanks @azasypkin! I addressed all feedback in 74639ce

jeramysoucy and others added 5 commits October 21, 2024 11:57
…t --include-path /api/status --include-path /api/alerting/rule/ --include-path /api/alerting/rules --include-path /api/actions --include-path /api/security/role --include-path /api/spaces --include-path /api/fleet --update'
@jeramysoucy jeramysoucy enabled auto-merge (squash) October 21, 2024 16:17
@jeramysoucy jeramysoucy merged commit c73bfd2 into elastic:main Oct 21, 2024
20 checks passed
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
serverless 14.1KB 14.1KB +1.0B

History

cc @jeramysoucy

jeramysoucy added a commit that referenced this pull request Nov 5, 2024
…#198827)

## Summary

[#195584](#195584) inadvertently
removed the cleanup of created roles during testing. This PR restores
the missing call to clean up any created roles.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! v9.0.0
Projects
None yet