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

Update go mod ref #2

Merged
merged 11 commits into from
Dec 20, 2023
Merged

Conversation

jbutlerdev
Copy link
Contributor

Update hashicorp/vault references to point to lf-edge/openbao

This will currently fail to build as the github.com/lf-edge/openbao/vault/hcp_link/proto needs to exist with the updated go.mod file before the go.sum file can be properly updated

@jbutlerdev
Copy link
Contributor Author

#1

@Scorpil
Copy link
Contributor

Scorpil commented Nov 29, 2023

Hey @jbutlerdev, hcp-link is still MPL 2.0, couldn't we just use it directly?

@jbutlerdev
Copy link
Contributor Author

Hey @jbutlerdev, hcp-link is still MPL 2.0, couldn't we just use it directly?

That's correct, however since the module is hosted within the vault repository and therefor also within the openbao repository I thought it made sense to point to our forked version. Do you think we should still be pointing to vault for this?

@Scorpil
Copy link
Contributor

Scorpil commented Nov 29, 2023

Oh it's in the vault repo... Nevermind then, I assumed it's pointing to the separate repo, my bad. You're right, we should keep it as is.

@Scorpil
Copy link
Contributor

Scorpil commented Nov 30, 2023

Alright, to test the branch I've added

replace github.com/lf-edge/openbao/vault/hcp_link/proto => ./vault/hcp_link/proto

to go.mod, this allows me to progress yet build fails on a compile step with

# github.com/lf-edge/openbao/helper/testhelpers/teststorage
helper/testhelpers/teststorage/teststorage.go:264:10: cannot use logicalKv.Factory (value of type func(ctx "context".Context, conf *"github.com/hashicorp/vault/sdk/logical".BackendConfig) ("github.com/hashicorp/vault/sdk/logical".Backend, error)) as "github.com/lf-edge/openbao/sdk/logical".Factory value in map literal
# github.com/lf-edge/openbao/helper/builtinplugins
helper/builtinplugins/registry.go:102:26: cannot use credAliCloud.Factory (value of type func(ctx "context".Context, conf *"github.com/hashicorp/vault/sdk/logical".BackendConfig) ("github.com/hashicorp/vault/sdk/logical".Backend, error)) as "github.com/lf-edge/openbao/sdk/logical".Factory value in struct literal

... and more of the same error...

Looks like logicalKv refers to "github.com/hashicorp/vault-plugin-secrets-kv" which of internally refers to

	github.com/hashicorp/vault/api v1.9.2
	github.com/hashicorp/vault/sdk v0.10.0

Causing the incompatibility. Any idea how to fix this short of forking "github.com/hashicorp/vault-plugin-secrets-kv"?

@jbutlerdev
Copy link
Contributor Author

Good call on the replace statement. I'll give this a shot today and get back to you

@naphelps
Copy link
Member

.../vault/api and .../vault/sdk both are still currently using MPL 2.0 license still. Short-term these references should not need to be changed.

Long-term if we needed to fork some of these plugins we would need to potentially work with OpenTofu to see if they use the same plugins or not, and what org the repo would fall under.

@naphelps
Copy link
Member

Make sure all commits use -s or --signoff for sign off.

@Scorpil
Copy link
Contributor

Scorpil commented Dec 4, 2023

@naphelps that was my original thinking as well (see hcp_link discussion above). However, as @jbutlerdev pointed out those modules are already forked (they're in this repo), so pointing to vault repo would be a bit dirty: first commiter who needs to modify those crucial sub-modules will be forced to solve the issue...

Besides, in this particular case it's not quite possible because the dependency is transitive: vault-plugin-secrets-kv improts values from hashicorp's vault, so when our codebase tries to use those - type mismatch happens.

@cipherboy
Copy link
Member

cipherboy commented Dec 4, 2023

I think using the replace operation and forking the plugins makes sense. They're external (in the sense that they're in a separate repository), but OpenBao (once built) views them as builtin plugins, not "external" plugins (which are a separate concept referring to plugin built as a separate executable, e.g., like vault-acme would be).

Forking allows us to version them all consistently along with the main executable. We could choose to pull these plugins in-tree or do the opposite and separate out all built-in plugins to be separate repositories.

@naphelps
Copy link
Member

naphelps commented Dec 4, 2023

Do we have a known good list of plugins that need to be additionally forked?

@cipherboy
Copy link
Member

cipherboy commented Dec 4, 2023

@naphelps said:

Do we have a known good list of plugins that need to be additionally forked?

This should be correct:

$ grep 'hashicorp/vault-plugin-' ./go.mod
	github.com/hashicorp/vault-plugin-auth-alicloud v0.15.0
	github.com/hashicorp/vault-plugin-auth-azure v0.16.0
	github.com/hashicorp/vault-plugin-auth-centrify v0.15.1
	github.com/hashicorp/vault-plugin-auth-cf v0.15.0
	github.com/hashicorp/vault-plugin-auth-gcp v0.16.0
	github.com/hashicorp/vault-plugin-auth-jwt v0.16.0
	github.com/hashicorp/vault-plugin-auth-kerberos v0.10.0
	github.com/hashicorp/vault-plugin-auth-kubernetes v0.16.0
	github.com/hashicorp/vault-plugin-auth-oci v0.14.0
	github.com/hashicorp/vault-plugin-database-couchbase v0.9.2
	github.com/hashicorp/vault-plugin-database-elasticsearch v0.13.2
	github.com/hashicorp/vault-plugin-database-mongodbatlas v0.10.0
	github.com/hashicorp/vault-plugin-database-redis v0.2.1
	github.com/hashicorp/vault-plugin-database-redis-elasticache v0.2.1
	github.com/hashicorp/vault-plugin-database-snowflake v0.9.0
	github.com/hashicorp/vault-plugin-mock v0.16.1
	github.com/hashicorp/vault-plugin-secrets-ad v0.16.0
	github.com/hashicorp/vault-plugin-secrets-alicloud v0.15.0
	github.com/hashicorp/vault-plugin-secrets-azure v0.16.1
	github.com/hashicorp/vault-plugin-secrets-gcp v0.16.0
	github.com/hashicorp/vault-plugin-secrets-gcpkms v0.15.0
	github.com/hashicorp/vault-plugin-secrets-kubernetes v0.5.0
	github.com/hashicorp/vault-plugin-secrets-kv v0.15.0
	github.com/hashicorp/vault-plugin-secrets-mongodbatlas v0.10.2
	github.com/hashicorp/vault-plugin-secrets-openldap v0.11.1
	github.com/hashicorp/vault-plugin-secrets-terraform v0.7.1

Only one I'm not really sure about is github.com/hashicorp/vault-plugin-mock.

@Scorpil
Copy link
Contributor

Scorpil commented Dec 4, 2023

Should be easy enough to comment them out for initial fork though, then take them up one-by-one. plugin-secrets-kv is used in many tests so it's the only one that's a must, imo.

@joewxboy
Copy link
Contributor

joewxboy commented Dec 4, 2023

Make sure all commits use -s or --signoff for sign off.

@jbutlerdev Could you go back and amend your commits to add the signoff flag?

@jbutlerdev
Copy link
Contributor Author

Make sure all commits use -s or --signoff for sign off.

@jbutlerdev Could you go back and amend your commits to add the signoff flag?

Aren't they signed off? They all show as verified

@naphelps
Copy link
Member

naphelps commented Dec 4, 2023

@jbutlerdev There is signing via GPG key, which you have done. Then, there is sign off, which requires adding the additional parameter during your commits. If you open each individual commit it should have a "signed-off-by: ..." line. We need both.

If you scroll down the checks below, the DCO bot checks for this specifically.

i.e.: 0b1971e

Signed-off-by: Jeremiah Butler <jeremiah.butler@ibm.com>
Signed-off-by: Jeremiah Butler <jeremiah.butler@ibm.com>
CONTRIBUTING.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
api/README.md Outdated Show resolved Hide resolved
api/auth/kubernetes/go.sum Outdated Show resolved Hide resolved
api/auth/ldap/go.mod Outdated Show resolved Hide resolved
api/auth/ldap/go.mod Outdated Show resolved Hide resolved
api/go.mod Outdated Show resolved Hide resolved
api/auth/userpass/go.mod Outdated Show resolved Hide resolved
api/auth/userpass/go.mod Outdated Show resolved Hide resolved
api/auth/userpass/go.sum Outdated Show resolved Hide resolved
api/auth/ldap/go.sum Outdated Show resolved Hide resolved
api/auth/ldap/go.sum Outdated Show resolved Hide resolved
api/auth/userpass/go.sum Outdated Show resolved Hide resolved
api/README.md Outdated Show resolved Hide resolved
changelog/note.tmpl Outdated Show resolved Hide resolved
Copy link
Member

@naphelps naphelps left a comment

Choose a reason for hiding this comment

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

Missed some of the lf-edge/openbao repo renames to openbao/openbao. 43 occurrences.

Signed-off-by: Jeremiah Butler <jeremiah.butler@ibm.com>
Signed-off-by: Jeremiah Butler <jeremiah.butler@ibm.com>
Signed-off-by: Jeremiah Butler <jeremiah.butler@ibm.com>
Signed-off-by: Jeremiah Butler <jeremiah.butler@ibm.com>
Signed-off-by: Jeremiah Butler <jeremiah.butler@ibm.com>
Signed-off-by: Jeremiah Butler <jeremiah.butler@ibm.com>
Signed-off-by: Jeremiah Butler <jeremiah.butler@ibm.com>
Signed-off-by: Jeremiah Butler <jeremiah.butler@ibm.com>
@naphelps naphelps merged commit bb9dc52 into openbao:development Dec 20, 2023
7 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants