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

add LicenseState() to SystemView interface #27930

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

thyton
Copy link
Contributor

@thyton thyton commented Jul 31, 2024

Description

Currently, enterprise plugins can be only run as builtin plugins. To allow enterprise plugins to run as external plugins, we need a way to validate Vault license. This PR adds LicenseState() to SystemView so that external plugins can request license state over gRPC, following a similar work done in #24929.

The new method can replace the current approach of using addLicenseCallback() to assign the plugin's Backend.LicenseState(). Whether the plugin is run as a builtin plugin or an external plugin, the plugin backend can use System().LicenseState(). If the plugin is no longer an enterprise plugin, we can simply remove the license check on the plugin repo.

func (b *Backend) HandleExistenceCheck(ctx context.Context, req *logical.Request) (checkFound bool, exists bool, err error) {
	// check license state using vault-licensing then feature
	licenseState, err := b.Backend.System().LicenseState()
	if err != nil {
 	    return false, false, errutil.UserError{Err: "license check failed, rejecting request"}
        }

	isValidState := license.IsValidState(licenseState)
        if isValidState || b.Backend.HasFeature(license.Features(entlicense.FeatureKeyManagementSecretsEngine)) {
	    return false, false, errutil.UserError{Err: "license check failed, rejecting request"}
	}

	// handle existence check
}

func (b *Backend) HandleRequest(ctx context.Context, req *logical.Request) (*logical.Response, error) {
	// check license state using vault-licensing then feature
	licenseState, err := b.Backend.System().LicenseState()
	if err != nil {
 	    return false, false, errutil.UserError{Err: "license check failed, rejecting request"}
        }

	isValidState := license.IsValidState(licenseState)
        if isValidState || b.Backend.HasFeature(license.Features(entlicense.FeatureKeyManagementSecretsEngine)) {
	    return false, false, errutil.UserError{Err: "license check failed, rejecting request"}
        }

        // handle request
}

Test output shows Vault with new plugin sdk works with older plugins with previous plugin sdk

root@6fa90bb39cbf:/vault_1.18.0# uname -a 
Linux 6fa90bb39cbf 6.6.31-linuxkit #1 SMP Thu May 23 08:36:57 UTC 2024 aarch64 aarch64 aarch64 GNU/Linux
root@6fa90bb39cbf:/vault_1.18.0# ./vault version
Vault v1.18.0-beta1 ('4b69030a147e84baa31b2b8d0db4784d5f430d08'), built 2024-08-12T10:20:51Z

root@6fa90bb39cbf:/vault_1.18.0# sha256sum ../plugins/vault-plugin-secrets-kv 
cd64c98f2695e13c4364650b53cb51406e699ee5fc2db67025f669f5d4c5c5d6  ../plugins/vault-plugin-secrets-kv
root@6fa90bb39cbf:/vault_1.18.0# ./vault plugin register -sha256=cd64c98f2695e13c4364650b53cb51406e699ee5fc2db67025f669f5d4c5c5d6 -version=v0.17.0 secret vault-plugin-secrets-kv
Success! Registered plugin: vault-plugin-secrets-kv

root@6fa90bb39cbf:/vault_1.18.0# ./vault plugin list | grep secrets-kv
vault-plugin-secrets-kv              secret      v0.17.0
root@6fa90bb39cbf:/vault_1.18.0# ./vault secrets enable -path=my-secret vault-plugin-secrets-kv    
Success! Enabled the vault-plugin-secrets-kv secrets engine at: my-secret/

root@6fa90bb39cbf:/vault_1.18.0# ./vault secrets list -format=json
...
  "my-secret/": {
    "uuid": "80e47a38-1c89-1d86-abc6-d088ceb759af",
    "type": "vault-plugin-secrets-kv",
    "description": "",
    "accessor": "vault-plugin-secrets-kv_b99ecf5f",
    "config": {
      "default_lease_ttl": 0,
      "max_lease_ttl": 0,
      "force_no_cache": false
    },
    "options": null,
    "local": false,
    "seal_wrap": false,
    "external_entropy_access": false,
    "plugin_version": "v0.17.0",
    "running_plugin_version": "v0.17.0",
    "running_sha256": "cd64c98f2695e13c4364650b53cb51406e699ee5fc2db67025f669f5d4c5c5d6",
    "deprecation_status": ""
  },
...

TODO only if you're a HashiCorp employee

  • Labels: If this PR is the CE portion of an ENT change, and that ENT change is
    getting backported to N-2, use the new style backport/ent/x.x.x+ent labels
    instead of the old style backport/x.x.x labels.
  • Labels: If this PR is a CE only change, it can only be backported to N, so use
    the normal backport/x.x.x label (there should be only 1).
  • ENT Breakage: If this PR either 1) removes a public function OR 2) changes the signature
    of a public function, even if that change is in a CE file, double check that
    applying the patch for this PR to the ENT repo and running tests doesn't
    break any tests. Sometimes ENT only tests rely on public functions in CE
    files.
  • Jira: If this change has an associated Jira, it's referenced either
    in the PR description, commit message, or branch name.
  • RFC: If this change has an associated RFC, please link it in the description.
  • ENT PR: If this change has an associated ENT PR, please link it in the
    description. Also, make sure the changelog is in this PR, not in your ENT PR.

@github-actions github-actions bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Jul 31, 2024
@thyton thyton requested a review from fairclothjm July 31, 2024 23:22
@thyton thyton added this to the 1.18.0-rc milestone Jul 31, 2024
Copy link

github-actions bot commented Jul 31, 2024

CI Results:
All Go tests succeeded! ✅

Copy link

github-actions bot commented Jul 31, 2024

Build Results:
All builds succeeded! ✅

@@ -226,6 +226,18 @@ func (s *gRPCSystemViewClient) GenerateIdentityToken(ctx context.Context, req *p
}, nil
}

func (s *gRPCSystemViewClient) HasLicense() (bool, error) {
reply, err := s.client.HasLicense(context.Background(), &pb.Empty{})
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible for the caller to pass the context instead of using context.Background() here?

@@ -479,3 +479,12 @@ func (d dynamicSystemView) GenerateIdentityToken(ctx context.Context, req *plugi
TTL: ttl,
}, nil
}

func (d dynamicSystemView) HasLicense() (bool, error) {
licenseState, err := d.core.EntGetLicenseState()
Copy link
Contributor

Choose a reason for hiding this comment

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

On the one hand, I like this approach because it is simple. It does not require us to update https://github.com/hashicorp/vault-licensing and other places in Vault core's code to add a new ent plugin.

On the other hand, it sidesteps the current paradigm of explicitly allowlisting each feature/plugin. So I am not sure about this. I am having trouble coming up with any downsides to your current approach other than we don't have fine-grained control. Maybe we should get more feedback from our team?

Copy link
Contributor Author

@thyton thyton Aug 1, 2024

Choose a reason for hiding this comment

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

it sidesteps the current paradigm of explicitly allowlisting each feature/plugin

I agree. Along this line, I wonder, if any point we have different types/tiers of licensing, HasLicense() will need to be changed. This makes me think HasFeature() of SystemView interface, currently left unimplemented, could solve the fine-grained control in both situations.

@thyton thyton requested a review from a team August 1, 2024 16:47
@thyton thyton force-pushed the VAULT-28732-add-hasLicense-to-systemView branch from 2d28084 to be0d397 Compare August 10, 2024 01:30
@thyton thyton changed the title add HasLicense() to SystemView interface add LicenseState() to SystemView interface Aug 12, 2024
@thyton thyton requested a review from ccapurso August 12, 2024 16:15
@thyton thyton removed this from the 1.18.0-rc milestone Sep 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants