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 KMS readme with vault quick start guide #6747

Merged
merged 1 commit into from
Nov 5, 2018

Conversation

poornas
Copy link
Contributor

@poornas poornas commented Nov 1, 2018

Description

Adding a quick start guide to configure Vault for use as KMS with minio server

Motivation and Context

Helps guide new users

Regression

No

How Has This Been Tested?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added unit tests to cover my changes.
  • I have added/updated functional tests in mint. (If yes, add mint PR # here: )
  • All new and existing tests passed.

@codecov
Copy link

codecov bot commented Nov 1, 2018

Codecov Report

Merging #6747 into master will increase coverage by 0.71%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6747      +/-   ##
==========================================
+ Coverage   52.25%   52.97%   +0.71%     
==========================================
  Files         269      269              
  Lines       42176    42986     +810     
==========================================
+ Hits        22039    22771     +732     
+ Misses      18199    18194       -5     
- Partials     1938     2021      +83
Impacted Files Coverage Δ
pkg/madmin/heal-commands.go 39.69% <0%> (-3.79%) ⬇️
cmd/gateway-main.go 16.08% <0%> (-0.09%) ⬇️
cmd/notification.go 14.02% <0%> (ø) ⬆️
cmd/fs-v1-helpers.go 70.03% <0%> (+0.91%) ⬆️
cmd/xl-sets.go 57.35% <0%> (+1.5%) ⬆️
cmd/bucket-handlers.go 62.09% <0%> (+3.12%) ⬆️
cmd/handler-utils.go 78.35% <0%> (+3.99%) ⬆️
cmd/common-main.go 17.26% <0%> (+4.24%) ⬆️
cmd/admin-handlers.go 25.95% <0%> (+4.67%) ⬆️
cmd/admin-heal-ops.go 56.09% <0%> (+6.44%) ⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3f19ea9...80ece8e. Read the comment docs.

@harshavardhana
Copy link
Member

We don't need a separate vault quick start guide, we should simply add it in the same documentation in the prerequisite section.

@poornas poornas force-pushed the fixvaultdocs branch 2 times, most recently from b0540bc to 2272500 Compare November 1, 2018 19:50
docs/kms/README.md Outdated Show resolved Hide resolved
docs/kms/README.md Outdated Show resolved Hide resolved
docs/kms/README.md Outdated Show resolved Hide resolved
vault auth enable approle # enable approle style auth
vault secrets enable transit # enable transit secrets engine
vault write -f transit/keys/my-minio-key #define a encryption key-ring for the transit path
vault policy write minio-policy ~/code/src/github.com/minio/vaultpolicy.hcl #define a policy for AppRole to access transit path
Copy link
Contributor

Choose a reason for hiding this comment

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

Should amend to a shorter path to the sample file (from the heredoc it implies pwd?)

vault secrets enable transit # enable transit secrets engine
vault write -f transit/keys/my-minio-key #define a encryption key-ring for the transit path
vault policy write minio-policy ~/code/src/github.com/minio/vaultpolicy.hcl #define a policy for AppRole to access transit path
vault write auth/approle/role/my-role token_num_uses=0 secret_id_num_uses=0 period=60s # period indicates it is renewable if tok renewed before the period is over
Copy link
Contributor

Choose a reason for hiding this comment

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

"...if token is renewed..."

@@ -14,6 +14,43 @@ Vault as Key Management System requires following to be configured in Vault
- AppRole based authentication with read/update policy for transit backend. In particular, read and update policy
are required for the generate data key endpoint and decrypt key endpoint.

Here is a sample quick start to configuring vault with a transit backend and Approle with correct policy
Copy link
Contributor

Choose a reason for hiding this comment

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

start to configuring vault => start for configuring vault
IMHO

@poornas
Copy link
Contributor Author

poornas commented Nov 2, 2018

@eco-minio && @ebozduman - updated PR.

Copy link
Contributor

@eco-minio eco-minio left a comment

Choose a reason for hiding this comment

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

objects.This document explains how to configure Minio with Vault as KMS.
objects. This document explains how to configure Minio with Vault as KMS.

@poornas poornas force-pushed the fixvaultdocs branch 2 times, most recently from 9e12bd0 to 8ee6ff7 Compare November 2, 2018 23:48
@poornas
Copy link
Contributor Author

poornas commented Nov 2, 2018

@ebozduman , addressed your feedback

are required for the generate data key endpoint and decrypt key endpoint.
- [transit](https://www.vaultproject.io/docs/secrets/transit/index.html) backend configured with a named encryption key-ring
- [AppRole](https://www.vaultproject.io/docs/auth/approle.html) based authentication with read/update policy for transit backend. In particular, read and update policy
are required for the Generate Data Key](https://www.vaultproject.io/api/secret/transit/index.html#generate-data-key) endpoint and [Decrypt Data](https://www.vaultproject.io/api/secret/transit/index.html#decrypt-data) endpoint.
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing [ in ...for the Generate Data Key](...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks,fixed.

@minio-ops
Copy link

Mint Automation

Test Result
mint-tls.sh ✔️
mint-xl.sh ✔️
mint-gateway-nas.sh ✔️
mint-large-bucket.sh ✔️
mint-worm.sh ✔️
mint-fs.sh ✔️
mint-dist-xl.sh ✔️

Copy link
Contributor

@ebozduman ebozduman left a comment

Choose a reason for hiding this comment

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

LGTM

@ebozduman
Copy link
Contributor

I noticed there is travis failure after giving my LGTM.
I don't think the failure is related to this PR, but it is about:

--- FAIL: TestAPICopyObjectPartHandler (1.73s)
	object-handlers_test.go:1767: Test 1: FS: Expected the response status to be `200`, but instead found `500`

@poornas
Copy link
Contributor Author

poornas commented Nov 5, 2018

@ebozduman , travis passed on restart

Copy link
Contributor

@eco-minio eco-minio left a comment

Choose a reason for hiding this comment

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

LGTM

@kannappanr kannappanr merged commit eb1f9c9 into minio:master Nov 5, 2018
poornas added a commit to poornas/minio that referenced this pull request Nov 28, 2018
@poornas poornas deleted the fixvaultdocs branch June 13, 2022 22:40
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.

6 participants