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

PKI: Remove references to managed keys and enterprise no-ops functions #169

Conversation

Gabrielopesantos
Copy link
Contributor

Description

This PR is a follow-up to #142, it removes the managed_key_util.go file from PKI, with no-ops enterprise functions and all of these functions.
Besides that, it also removes all references and usages of managed keys in the engine and SDK.

P.S: This PR has to be rebased once #142 is merged.

@Gabrielopesantos Gabrielopesantos force-pushed the gabrielopesantos/remove-enterprise-no-ops branch 3 times, most recently from 675b33e to 7e54c6f Compare March 6, 2024 22:42
@Gabrielopesantos Gabrielopesantos marked this pull request as ready for review March 6, 2024 22:45
Copy link
Member

@cipherboy cipherboy left a comment

Choose a reason for hiding this comment

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

If you don't mind doing an interactive rebase, it'd be nice to drop both 46b9d99c7d5a261d81e9fead18c11947f4e24df9 and 4ef68b486f9828055a69c8af9407b21ca22af2ff, so we don't remove-and-re-add in the same PR since it won't be squashed. :-)

Otherwise I think this looks good, thanks @Gabrielopesantos!!

@Gabrielopesantos Gabrielopesantos force-pushed the gabrielopesantos/remove-enterprise-no-ops branch from c2d48fc to 9b178ba Compare March 7, 2024 18:06
@Gabrielopesantos
Copy link
Contributor Author

Gabrielopesantos commented Mar 7, 2024

If you don't mind doing an interactive rebase, it'd be nice to drop both 46b9d99c7d5a261d81e9fead18c11947f4e24df9 and 4ef68b486f9828055a69c8af9407b21ca22af2ff, so we don't remove-and-re-add in the same PR since it won't be squashed. :-)

Otherwise I think this looks good, thanks @Gabrielopesantos!!

Sure, I couldn't find the objects (commits) you mentioned and rebased the branch with the main. Is that what you meant?

EDIT: I also see the "Check changelog" failing, as expected, is that already being enforced and do I need to add a changelog file to the PR?

@cipherboy
Copy link
Member

cipherboy commented Mar 9, 2024

@Gabrielopesantos Feel free to skip the CL, I'm considering this all part of the initial "fork" changeset. I don't have labeling permissions and Nathan has been ignoring it :-)

Sorry, not sure how I gave you such bogus commits lol. I meant the first and the last ones:

  • a001bd5c3d6313a5c9567f8fed95d58a91dc5aa3 / Remove IsEnterprise references (modifies a single Transit file)
  • 9b178ba8ea1e596c948ae21d22d87594cc38aa2a / Revert deleted import in transit test (undoes that modification).

@Gabrielopesantos Gabrielopesantos force-pushed the gabrielopesantos/remove-enterprise-no-ops branch 2 times, most recently from 0eafe99 to b068be8 Compare March 10, 2024 12:11
@Gabrielopesantos
Copy link
Contributor Author

Gabrielopesantos commented Mar 10, 2024

@Gabrielopesantos Feel free to skip the CL, I'm considering this all part of the initial "fork" changeset. I don't have labeling permissions and Nathan has been ignoring it :-)

Sorry, not sure how I gave you such bogus commits lol. I meant the first and the last ones:

  • a001bd5c3d6313a5c9567f8fed95d58a91dc5aa3 / Remove IsEnterprise references (modifies a single Transit file)
  • 9b178ba8ea1e596c948ae21d22d87594cc38aa2a / Revert deleted import in transit test (undoes that modification).

I did not see that change was introduced in the first commit. Addressed. 👍

Copy link
Member

@cipherboy cipherboy left a comment

Choose a reason for hiding this comment

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

Thank you @Gabrielopesantos!! Looks good to me :-) Want to try unified CRLs next?

@Gabrielopesantos Gabrielopesantos force-pushed the gabrielopesantos/remove-enterprise-no-ops branch from b068be8 to db1213d Compare March 12, 2024 22:13
@naphelps naphelps added this pull request to the merge queue Mar 13, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Mar 13, 2024
@naphelps
Copy link
Member

@Gabrielopesantos As soon as you rebase this with updates, I will merge this one in. Thanks.

@Gabrielopesantos Gabrielopesantos force-pushed the gabrielopesantos/remove-enterprise-no-ops branch from db1213d to a196bc8 Compare March 13, 2024 23:28
Signed-off-by: Gabriel Santos <gabrielopesantos97@gmail.com>
Signed-off-by: Gabriel Santos <gabrielopesantos97@gmail.com>
Signed-off-by: Gabriel Santos <gabrielopesantos97@gmail.com>
Signed-off-by: Gabriel Santos <gabrielopesantos97@gmail.com>
@naphelps naphelps force-pushed the gabrielopesantos/remove-enterprise-no-ops branch from a196bc8 to d08f3d6 Compare March 14, 2024 00:22
@naphelps naphelps merged commit 7bd8c5d into openbao:main Mar 14, 2024
71 of 79 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

3 participants