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

Ent only ADP Metrics #21681

Merged
merged 16 commits into from
Jul 10, 2023
Merged

Ent only ADP Metrics #21681

merged 16 commits into from
Jul 10, 2023

Conversation

divyaac
Copy link
Contributor

@divyaac divyaac commented Jul 7, 2023

@divyaac divyaac requested a review from mpalmi July 7, 2023 19:17
@github-actions github-actions bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Jul 7, 2023
vault/core.go.rej Outdated Show resolved Hide resolved
Copy link
Contributor

@mpalmi mpalmi left a comment

Choose a reason for hiding this comment

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

Looking good! Just a few things to fix up and then we can merge.

changelog/_21681.txt Outdated Show resolved Hide resolved
vault/mount.go Show resolved Hide resolved
vault/core.go Outdated
@@ -3234,28 +3235,7 @@ func (c *Core) isMountable(ctx context.Context, entry *MountEntry, pluginType co
// isMountEntryBuiltin determines whether a mount entry is associated with a
// builtin of the specified plugin type.
func (c *Core) isMountEntryBuiltin(ctx context.Context, entry *MountEntry, pluginType consts.PluginType) bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

If you're going to have separate implementations for OSS/ENT, you'd want to do this in core_util.go.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not sure how this happened - but removed this impl because this wasn't a change in ent either.

@@ -0,0 +1,3 @@
```release-note:improvement
sys/metrics: Adds a gauge metric that tracks whether enterprise builtin secret plugins are enabled.
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like we lost the (enterprise) decorator here.

@@ -283,6 +283,11 @@ func (r *registry) DeprecationStatus(name string, pluginType consts.PluginType)
return consts.Unknown, false
}

// IsBuiltinEntPlugin checks whether the plugin is an enterprise only builtin plugin
func (r *registry) IsBuiltinEntPlugin(name string, pluginType consts.PluginType) bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you're going to have to handle both OSS and ENT here. This file is in both OSS and ENT.

@@ -26,6 +26,8 @@ import (
"github.com/mitchellh/go-testing-interface"
)

var externalPlugins = []string{""}
Copy link
Contributor

Choose a reason for hiding this comment

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

This is also agnostic of OSS/ENT, so we'll have to change move this elsewhere.

Copy link
Contributor

@mpalmi mpalmi left a comment

Choose a reason for hiding this comment

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

Looks great!

@divyaac divyaac added this to the 1.15 milestone Jul 10, 2023
@divyaac divyaac merged commit 9ace875 into main Jul 10, 2023
98 checks passed
@divyaac divyaac deleted the VAULT-16547 branch July 10, 2023 17:28
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.

None yet

2 participants