Skip to content

Add license checking during recover from snapshot#78928

Closed
fcofdez wants to merge 7 commits intoelastic:masterfrom
fcofdez:recovery-license-check
Closed

Add license checking during recover from snapshot#78928
fcofdez wants to merge 7 commits intoelastic:masterfrom
fcofdez:recovery-license-check

Conversation

@fcofdez
Copy link
Copy Markdown
Contributor

@fcofdez fcofdez commented Oct 11, 2021

This commit introduces a new plug-in, LicenseCheckerPlugin, that allows
checking if a particular feature is allowed or not depending on the
current license in server code. The node won't start unless there's
a single LicenseCheckerPlugin.

Additionally, snapshot based recoveries fallback to regular source
based recoveries if the current license doesn't allow to run that
feature.

This commit introduces a new plug-in LicenseCheckerPlugin that allows
checking if a particular feature is allowed or not depending on the
current license in server code. The node won't start unless there's
a single LicenseCheckerPlugin.
@fcofdez fcofdez force-pushed the recovery-license-check branch from 7fea984 to 0c0532d Compare October 13, 2021 16:22
@fcofdez fcofdez marked this pull request as ready for review October 13, 2021 17:14
@fcofdez fcofdez added the Team:Distributed Meta label for distributed team. label Oct 13, 2021
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-distributed (Team:Distributed)

@fcofdez fcofdez added :Distributed/Recovery Anything around constructing a new shard, either from a local or a remote source. v7.16.0 labels Oct 13, 2021
@fcofdez
Copy link
Copy Markdown
Contributor Author

fcofdez commented Oct 13, 2021

Maybe it makes sense to add someone from delivery or core as reviewers too?

Copy link
Copy Markdown
Contributor

@henningandersen henningandersen left a comment

Choose a reason for hiding this comment

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

I left a few minor comments but otherwise this looks good.

I should be interested in whether @rjernst has input to how we plug this in?

Copy link
Copy Markdown
Contributor

@henningandersen henningandersen left a comment

Choose a reason for hiding this comment

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

LGTM.

@fcofdez fcofdez added the :Security/License License functionality for commercial features label Oct 14, 2021
@elasticmachine elasticmachine added the Team:Security Meta label for security team label Oct 14, 2021
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-security (Team:Security)

@rjernst rjernst self-requested a review October 14, 2021 21:16
Copy link
Copy Markdown
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

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

I don't think we should add license checking to server. It would blur the lines of what is licensed vs unlicensed code. Instead, I wonder about the SnapshotsRecoveryPlannerService in general:

final RecoveryPlannerService recoveryPlannerService = new SnapshotsRecoveryPlannerService(shardSnapshotsService);

This is an excerpt from Node construction. It seems to me the RecoveryPlannerService should be what is pluggable, then the implementation could be in xpack and have normal license checks, when necessary.

@fcofdez
Copy link
Copy Markdown
Contributor Author

fcofdez commented Oct 26, 2021

Closing this in favor of #79637

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

Labels

:Distributed/Recovery Anything around constructing a new shard, either from a local or a remote source. :Security/License License functionality for commercial features Team:Distributed Meta label for distributed team. Team:Security Meta label for security team v7.16.0 v8.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants