Conversation
kp import will attempt to read registry credentials from these environment variables as a fallback to docker/.config during `kp import`: * REGISTRY_USER * REGISTRY_PASSWORD
Examples: ``` # Passing multiple registry credentials using the _N suffix REGISTRY_USER_0=pablo REGISTRY_PASSWORD_0=picasso REGISTRY_URL_0=first-registry.io \ REGISTRY_USER_1=henri REGISTRY_PASSWORD_1=matisse REGISTRY_URL_1=another-registry.io \ kp import -f descriptor.yaml # Passing a single registry credential by omitting the _N suffix REGISTRY_USER=bob REGISTRY_PASSWORD=ross REGISTRY_URL=single-registry.io \ kp import -f descriptor.yaml ```
|
We will also need to add docs to the help message so that users know how to utilize this feature |
chenbh
left a comment
There was a problem hiding this comment.
I agree with #251 (comment) that this should apply to every command that interacts with a registry
|
|
Addressing this now... |
* Simplified access to new Keychain * Renamed env vars to: - KP_REGISTRY_HOSTNAME - KP_REGISTRY_USERNAME - KP_REGISTRY_PASSWORD * Removed superfluous keychain_helper wrapper and tests Signed-off-by: Nicholas Carlson <carlsonn@vmware.com>
914a3f8 to
2b67240
Compare
| EnvVarRegistryUrl = "KP_REGISTRY_HOSTNAME" | ||
| EnvVarRegistryUser = "KP_REGISTRY_USERNAME" | ||
| EnvVarRegistryPassword = "KP_REGISTRY_PASSWORD" |
There was a problem hiding this comment.
Do these need to be exported?
| package dockercreds | ||
|
|
||
| var NewCredHelperFromEnvVars = newCredHelperFromEnvVars |
There was a problem hiding this comment.
If I'm reading this correctly, the whole point of this file is to expose the newCredHelperFromEnvVars function for tests? In that case a better idea is to just use package dockercreds in cred_helper_test.go.
The way go works, this would expose the function to any tests, including ones from outside this module.
There was a problem hiding this comment.
FYI, I've found testing to be somewhat inconsistent across even files of the same project.
The rule of thumb is that if you purely want to test public functions, then you can use package my_module_test, if you want to test internals, then it's perfectly fine to use package my_module inside a *_test.go file
| it.After(func() { | ||
| require.NoError(t, os.Unsetenv(envVarRegistryUrl)) | ||
| require.NoError(t, os.Unsetenv(envVarRegistryUser)) | ||
| require.NoError(t, os.Unsetenv(envVarRegistryPassword)) | ||
| }) |
There was a problem hiding this comment.
You don't need to worry about cleaning up env vars since child processes cannot interact with the parent's env vars (other than inheriting a copy).
For example, go run-ing this program won't unset $HOME nor add $FOOBAR to the shell
package main
import "os"
func main() {
os.Setenv("FOOBAR", "BAZ")
os.Unsetenv("HOME")
}…port-env-vars-for-registry-auth-when-executing-kp-import Use env vars for registry credentials in kp import
kp import will attempt to read registry credentials from these environment variables as a fallback to docker/.config during
kp import.Examples: