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

feat(cli): new storage config input from file when creating a repo #2756

Merged
merged 4 commits into from Jul 27, 2023

Conversation

Shrekster
Copy link
Collaborator

@Shrekster Shrekster commented Feb 10, 2023

When creating a repo we are currently required to supply storage config secrets as CLI arguments. CLI commands often get logged which can accidentally expose these creds. This PR extends the from-config storage flags to be supplied at the time of creation with a dedicated --token-file option or --token-stdin option that reads from stdin.

echo "${kopia_string}" | kopia repository create from-config --token-stdin
kopia repository create from-config --token-file /path/to/token-file

@codecov
Copy link

codecov bot commented Feb 10, 2023

Codecov Report

Patch coverage: 87.09% and project coverage change: +0.02% 🎉

Comparison is base (83b88d8) 75.41% compared to head (fca1bf1) 75.43%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2756      +/-   ##
==========================================
+ Coverage   75.41%   75.43%   +0.02%     
==========================================
  Files         459      459              
  Lines       36424    36445      +21     
==========================================
+ Hits        27468    27493      +25     
+ Misses       7041     7036       -5     
- Partials     1915     1916       +1     
Files Changed Coverage Δ
cli/command_repository_create.go 73.60% <ø> (-0.21%) ⬇️
cli/storage_providers.go 100.00% <ø> (ø)
cli/command_repository_connect_from_config.go 73.21% <85.71%> (+25.99%) ⬆️
repo/token.go 67.85% <100.00%> (+14.01%) ⬆️

... and 4 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


sps StorageProviderServices
}

func (c *storageFromConfigFlags) Setup(sps StorageProviderServices, cmd *kingpin.CmdClause) {
cmd.Flag("file", "Path to the configuration file").StringVar(&c.connectFromConfigFile)
cmd.Flag("token", "Configuration token").StringVar(&c.connectFromConfigToken)
cmd.Flag("token-file", "Path to the configuration token file").StringVar(&c.connectFromTokenFile)
Copy link
Contributor

Choose a reason for hiding this comment

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

how about (also) adding a boolean token-stdin or something similar ? This would make it work on Windows where /dev/stdin is not available.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good idea, will do

Copy link

Choose a reason for hiding this comment

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

@jkowalski, According to my understanding, we want to introduce a flag token-stdin, which is basically of boolean type. If the value is false than user have to pass the token file path and if true than user have to pass the value of token thru stdin ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added the --token-stdin changes with the unit-test

@pavannd1 pavannd1 requested a review from julio-lopez May 18, 2023 20:21
@Shrekster Shrekster force-pushed the storage-config-provisioning-flag branch from c8ef5ff to 341b530 Compare July 26, 2023 23:26
@Shrekster Shrekster force-pushed the storage-config-provisioning-flag branch from 341b530 to 69aeba9 Compare July 26, 2023 23:29
@Shrekster Shrekster enabled auto-merge (squash) July 27, 2023 20:37
@Shrekster Shrekster disabled auto-merge July 27, 2023 20:37
@Shrekster Shrekster enabled auto-merge (squash) July 27, 2023 20:37
@Shrekster Shrekster disabled auto-merge July 27, 2023 20:37
@Shrekster Shrekster enabled auto-merge (squash) July 27, 2023 20:51
@Shrekster Shrekster disabled auto-merge July 27, 2023 20:51
@Shrekster Shrekster enabled auto-merge (squash) July 27, 2023 20:51
@Shrekster Shrekster requested a review from jkowalski July 27, 2023 21:20
@julio-lopez
Copy link
Collaborator

@Shrekster nit: modified the PR description to remove --password .... in order to better reflect the desired usage pattern.

@Shrekster Shrekster merged commit e1c44f7 into kopia:master Jul 27, 2023
28 checks passed
@Shrekster Shrekster deleted the storage-config-provisioning-flag branch July 27, 2023 21:41
@julio-lopez
Copy link
Collaborator

Thanks for the change!

@pavannd1
Copy link
Contributor

Thank you, @Shrekster! 🎉

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