kubernetes
/
git-sync
mirror of https://github.com/kubernetes/git-sync.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Add the idea of "env-flags" 

Env-flags are "flags" that can only be set by env var (see caveat below).
All of the real flags have a corresponding env-flag (kind of, but not
really).  The real goal was to deprecate `--password` but keep the env
var as a documented interface.

This does that (though --password still works) and updates the usage and
manual.

This allows some future work to follow the pattern.  We do not register
every CLI flag as an env-flag because the help text would be
duplicative.  This probably wants a wrapper API that allows declaring of
abstract flags, with CLI, env, or both sources.

Caveat:

ACTUALLY, these still have a flag, but the flag is specially named and
hidden.  This makes testing a little easier where passing flags is
handled well but env vars is not.
This commit is contained in:
Tim Hockin 2024-06-08 18:28:32 -07:00
parent aa0f015606
commit 28394608e8
4 changed files with 189 additions and 78 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

52
README.md
View File

@ -143,7 +143,8 @@ OPTIONS
Many options can be specified as either a commandline flag or an environment
variable, but flags are preferred because a misspelled flag is a fatal
error while a misspelled environment variable is silently ignored.
error while a misspelled environment variable is silently ignored. Some
options can only be specified as an environment variable.
--add-user, $GITSYNC_ADD_USER
Add a record to /etc/passwd for the current UID/GID. This is
@ -161,11 +162,12 @@ OPTIONS
--credential <string>, $GITSYNC_CREDENTIAL
Make one or more credentials available for authentication (see git
help credential). This is similar to --username and --password or
--password-file, but for specific URLs, for example when using
submodules. The value for this flag is either a JSON-encoded
object (see the schema below) or a JSON-encoded list of that same
object type. This flag may be specified more than once.
help credential). This is similar to --username and
$GITSYNC_PASSWORD or --password-file, but for specific URLs, for
example when using submodules. The value for this flag is either a
JSON-encoded object (see the schema below) or a JSON-encoded list
of that same object type. This flag may be specified more than
once.
Object schema:
- url: string, required
@ -294,16 +296,14 @@ OPTIONS
--one-time, $GITSYNC_ONE_TIME
Exit after one sync.
--password <string>, $GITSYNC_PASSWORD
$GITSYNC_PASSWORD
The password or personal access token (see github docs) to use for
git authentication (see --username). NOTE: for security reasons,
users should prefer --password-file or $GITSYNC_PASSWORD_FILE for
specifying the password.
git authentication (see --username). See also --password-file.
--password-file <string>, $GITSYNC_PASSWORD_FILE
The file from which the password or personal access token (see
github docs) to use for git authentication (see --username) will be
read.
read. See also $GITSYNC_PASSWORD.
--period <duration>, $GITSYNC_PERIOD
How long to wait between sync attempts. This must be at least
@ -376,8 +376,8 @@ OPTIONS
--username <string>, $GITSYNC_USERNAME
The username to use for git authentication (see --password-file or
--password). If more than one username and password is required
(e.g. with submodules), use --credential.
$GITSYNC_PASSWORD). If more than one username and password is
required (e.g. with submodules), use --credential.
-v, --verbose <int>, $GITSYNC_VERBOSE
Set the log verbosity level. Logs at this level and lower will be
@ -435,31 +435,31 @@ AUTHENTICATION
and "git@example.com:repo" will try to use SSH.
username/password
The --username (GITSYNC_USERNAME) and --password-file
(GITSYNC_PASSWORD_FILE) or --password (GITSYNC_PASSWORD) flags
will be used. To prevent password leaks, the --password-file flag
or GITSYNC_PASSWORD environment variable is almost always
preferred to the --password flag.
The --username ($GITSYNC_USERNAME) and $GITSYNC_PASSWORD or
--password-file ($GITSYNC_PASSWORD_FILE) flags will be used. To
prevent password leaks, the --password-file flag or
$GITSYNC_PASSWORD environment variable is almost always preferred
to the --password flag, which is deprecated.
A variant of this is --askpass-url (GITSYNC_ASKPASS_URL), which
A variant of this is --askpass-url ($GITSYNC_ASKPASS_URL), which
consults a URL (e.g. http://metadata) to get credentials on each
sync.
When using submodules it may be necessary to specify more than one
username and password, which can be done with --credential
(GITSYNC_CREDENTIAL). All of the username+password pairs, from
both --username/--password and --credential are fed into 'git
credential approve'.
($GITSYNC_CREDENTIAL). All of the username+password pairs, from
both --username/$GITSYNC_PASSWORD and --credential are fed into
'git credential approve'.
SSH
When an SSH transport is specified, the key(s) defined in
--ssh-key-file (GITSYNC_SSH_KEY_FILE) will be used. Users are
--ssh-key-file ($GITSYNC_SSH_KEY_FILE) will be used. Users are
strongly advised to also use --ssh-known-hosts
(GITSYNC_SSH_KNOWN_HOSTS) and --ssh-known-hosts-file
(GITSYNC_SSH_KNOWN_HOSTS_FILE) when using SSH.
($GITSYNC_SSH_KNOWN_HOSTS) and --ssh-known-hosts-file
($GITSYNC_SSH_KNOWN_HOSTS_FILE) when using SSH.
cookies
When --cookie-file (GITSYNC_COOKIE_FILE) is specified, the
When --cookie-file ($GITSYNC_COOKIE_FILE) is specified, the
associated cookies can contain authentication information.
HOOKS

101
env.go
View File

@ -17,11 +17,16 @@ limitations under the License.
package main
import (
"cmp"
"fmt"
"io"
"os"
"slices"
"strconv"
"strings"
"time"
"github.com/spf13/pflag"
)
func envString(def string, key string, alts ...string) string {
@ -30,12 +35,25 @@ func envString(def string, key string, alts ...string) string {
}
for _, alt := range alts {
if val := os.Getenv(alt); val != "" {
fmt.Fprintf(os.Stderr, "env %s has been deprecated, use %s instead\n", alt, key)
fmt.Fprintf(os.Stderr, "env $%s has been deprecated, use $%s instead\n", alt, key)
return val
}
}
return def
}
func envFlagString(key string, def string, usage string, alts ...string) *string {
registerEnvFlag(key, "string", usage)
val := envString(def, key, alts...)
// also expose it as a flag, for easier testing
flName := "__env__" + key
flHelp := "DO NOT SET THIS FLAG EXCEPT IN TESTS; use $" + key
newExplicitFlag(&val, flName, flHelp, pflag.String)
if err := pflag.CommandLine.MarkHidden(flName); err != nil {
fmt.Fprintf(os.Stderr, "FATAL: %v\n", err)
os.Exit(1)
}
return &val
}
func envStringArray(def string, key string, alts ...string) []string {
parse := func(s string) []string {
@ -47,7 +65,7 @@ func envStringArray(def string, key string, alts ...string) []string {
}
for _, alt := range alts {
if val := os.Getenv(alt); val != "" {
fmt.Fprintf(os.Stderr, "env %s has been deprecated, use %s instead\n", alt, key)
fmt.Fprintf(os.Stderr, "env $%s has been deprecated, use $%s instead\n", alt, key)
return parse(val)
}
}
@ -68,7 +86,7 @@ func envBoolOrError(def bool, key string, alts ...string) (bool, error) {
}
for _, alt := range alts {
if val := os.Getenv(alt); val != "" {
fmt.Fprintf(os.Stderr, "env %s has been deprecated, use %s instead\n", alt, key)
fmt.Fprintf(os.Stderr, "env $%s has been deprecated, use $%s instead\n", alt, key)
return parse(alt, val)
}
}
@ -98,7 +116,7 @@ func envIntOrError(def int, key string, alts ...string) (int, error) {
}
for _, alt := range alts {
if val := os.Getenv(alt); val != "" {
fmt.Fprintf(os.Stderr, "env %s has been deprecated, use %s instead\n", alt, key)
fmt.Fprintf(os.Stderr, "env $%s has been deprecated, use $%s instead\n", alt, key)
return parse(alt, val)
}
}
@ -128,7 +146,7 @@ func envFloatOrError(def float64, key string, alts ...string) (float64, error) {
}
for _, alt := range alts {
if val := os.Getenv(alt); val != "" {
fmt.Fprintf(os.Stderr, "env %s has been deprecated, use %s instead\n", alt, key)
fmt.Fprintf(os.Stderr, "env $%s has been deprecated, use $%s instead\n", alt, key)
return parse(alt, val)
}
}
@ -158,7 +176,7 @@ func envDurationOrError(def time.Duration, key string, alts ...string) (time.Dur
}
for _, alt := range alts {
if val := os.Getenv(alt); val != "" {
fmt.Fprintf(os.Stderr, "env %s has been deprecated, use %s instead\n", alt, key)
fmt.Fprintf(os.Stderr, "env $%s has been deprecated, use $%s instead\n", alt, key)
return parse(alt, val)
}
}
@ -173,3 +191,74 @@ func envDuration(def time.Duration, key string, alts ...string) time.Duration {
}
return val
}
// explicitFlag is a pflag.Value which only sets the real value if the flag is
// set to a non-zero-value.
type explicitFlag[T comparable] struct {
pflag.Value
realPtr *T
flagPtr *T
}
// newExplicitFlag allocates an explicitFlag
func newExplicitFlag[T comparable](ptr *T, name, usage string, fn func(name string, value T, usage string) *T) {
h := &explicitFlag[T]{
realPtr: ptr,
}
var zero T
h.flagPtr = fn(name, zero, usage)
fl := pflag.CommandLine.Lookup(name)
// wrap the original pflag.Value with our own
h.Value = fl.Value
fl.Value = h
}
func (h *explicitFlag[T]) Set(val string) error {
if err := h.Value.Set(val); err != nil {
return err
}
var zero T
if v := *h.flagPtr; v != zero {
*h.realPtr = v
}
return nil
}
// envFlag is like a flag in that it is declared with a type, validated, and
// shows up in help messages, but can only be set by env-var, not on the CLI.
// This is useful for things like passwords, which should not be on the CLI
// because it can be seen in `ps`.
type envFlag struct {
name string
typ string
help string
}
var allEnvFlags = []envFlag{}
// registerEnvFlag is internal. Use functions like envFlagString to actually
// create envFlags.
func registerEnvFlag(name, typ, help string) {
for _, ef := range allEnvFlags {
if ef.name == name {
fmt.Fprintf(os.Stderr, "FATAL: duplicate env var declared: %q\n", name)
os.Exit(1)
}
}
allEnvFlags = append(allEnvFlags, envFlag{name, typ, help})
}
// printEnvFlags prints "usage" for all registered envFlags.
func printEnvFlags(out io.Writer) {
width := 0
for _, ef := range allEnvFlags {
if n := len(ef.name); n > width {
width = n
}
}
slices.SortFunc(allEnvFlags, func(l, r envFlag) int { return cmp.Compare(l.name, r.name) })
for _, ef := range allEnvFlags {
fmt.Fprintf(out, "% *s %s %*s%s\n", width+2, ef.name, ef.typ, max(8, 32-width), "", ef.help)
}
}

92
main.go
View File

@ -141,7 +141,7 @@ func main() {
flVersion := pflag.Bool("version", false, "print the version and exit")
flHelp := pflag.BoolP("help", "h", false, "print help text and exit")
pflag.BoolVarP(flHelp, "__?", "?", false, "print help text and exit") // support -? as an alias to -h
pflag.BoolVarP(flHelp, "__?", "?", false, "") // support -? as an alias to -h
mustMarkHidden("__?")
flManual := pflag.Bool("man", false, "print the full manual and exit")
@ -230,9 +230,9 @@ func main() {
flUsername := pflag.String("username",
envString("", "GITSYNC_USERNAME", "GIT_SYNC_USERNAME"),
"the username to use for git auth")
flPassword := pflag.String("password",
envString("", "GITSYNC_PASSWORD", "GIT_SYNC_PASSWORD"),
"the password or personal access token to use for git auth (prefer --password-file or this env var)")
flPassword := envFlagString("GITSYNC_PASSWORD", "",
"the password or personal access token to use for git auth",
"GIT_SYNC_PASSWORD")
flPasswordFile := pflag.String("password-file",
envString("", "GITSYNC_PASSWORD_FILE", "GIT_SYNC_PASSWORD_FILE"),
"the file from which the password or personal access token for git auth will be sourced")
@ -280,30 +280,43 @@ func main() {
flDeprecatedBranch := pflag.String("branch", envString("", "GIT_SYNC_BRANCH"),
"DEPRECATED: use --ref instead")
mustMarkDeprecated("branch", "use --ref instead")
flDeprecatedChmod := pflag.Int("change-permissions", envInt(0, "GIT_SYNC_PERMISSIONS"),
"DEPRECATED: use --group-write instead")
mustMarkDeprecated("change-permissions", "use --group-write instead")
flDeprecatedDest := pflag.String("dest", envString("", "GIT_SYNC_DEST"),
"DEPRECATED: use --link instead")
mustMarkDeprecated("dest", "use --link instead")
flDeprecatedMaxSyncFailures := pflag.Int("max-sync-failures", envInt(0, "GIT_SYNC_MAX_SYNC_FAILURES"),
"DEPRECATED: use --max-failures instead")
mustMarkDeprecated("max-sync-failures", "use --max-failures instead")
flDeprecatedPassword := pflag.String("password", "", // the env vars are not deprecated
"DEPRECATED: use --password-file or $GITSYNC_PASSWORD instead")
mustMarkDeprecated("password", "use --password-file or $GITSYNC_PASSWORD instead")
flDeprecatedRev := pflag.String("rev", envString("", "GIT_SYNC_REV"),
"DEPRECATED: use --ref instead")
mustMarkDeprecated("rev", "use --ref instead")
_ = pflag.Bool("ssh", false,
"DEPRECATED: this flag is no longer necessary")
mustMarkDeprecated("ssh", "no longer necessary")
flDeprecatedSyncHookCommand := pflag.String("sync-hook-command", envString("", "GIT_SYNC_HOOK_COMMAND"),
"DEPRECATED: use --exechook-command instead")
mustMarkDeprecated("sync-hook-command", "use --exechook-command instead")
flDeprecatedTimeout := pflag.Int("timeout", envInt(0, "GIT_SYNC_TIMEOUT"),
"DEPRECATED: use --sync-timeout instead")
mustMarkDeprecated("timeout", "use --sync-timeout instead")
flDeprecatedV := pflag.Int("v", -1,
"DEPRECATED: use -v or --verbose instead")
mustMarkDeprecated("v", "use -v or --verbose instead")
flDeprecatedWait := pflag.Float64("wait", envFloat(0, "GIT_SYNC_WAIT"),
"DEPRECATED: use --period instead")
mustMarkDeprecated("wait", "use --period instead")
@ -316,9 +329,14 @@ func main() {
if msg != "" {
fmt.Fprintln(out, msg)
}
fmt.Fprintln(out, "Usage:")
fmt.Fprintf(out, "Usage: %s [FLAGS...]\n", filepath.Base(os.Args[0]))
fmt.Fprintln(out, "")
fmt.Fprintln(out, " FLAGS:")
pflag.CommandLine.SetOutput(out)
pflag.PrintDefaults()
fmt.Fprintln(out, "")
fmt.Fprintln(out, " ENVIRONMENT VARIABLES:")
printEnvFlags(out)
if msg != "" {
fmt.Fprintln(out, msg)
}
@ -492,12 +510,16 @@ func main() {
}
}
if *flDeprecatedPassword != "" {
log.V(0).Info("setting $GITSYNC_PASSWORD from deprecated --password")
*flPassword = *flDeprecatedPassword
}
if *flUsername != "" {
if *flPassword == "" && *flPasswordFile == "" {
fatalConfigError(log, true, "required flag: --password or --password-file must be specified when --username is specified")
fatalConfigError(log, true, "required flag: $GITSYNC_PASSWORD or --password-file must be specified when --username is specified")
}
if *flPassword != "" && *flPasswordFile != "" {
fatalConfigError(log, true, "invalid flag: only one of --password and --password-file may be specified")
fatalConfigError(log, true, "invalid flag: only one of $GITSYNC_PASSWORD and --password-file may be specified")
}
if u, err := url.Parse(*flRepo); err == nil { // it may not even parse as a URL, that's OK
if u.User != nil {
@ -506,7 +528,7 @@ func main() {
}
} else {
if *flPassword != "" {
fatalConfigError(log, true, "invalid flag: --password may only be specified when --username is specified")
fatalConfigError(log, true, "invalid flag: $GITSYNC_PASSWORD may only be specified when --username is specified")
}
if *flPasswordFile != "" {
fatalConfigError(log, true, "invalid flag: --password-file may only be specified when --username is specified")
@ -1807,7 +1829,7 @@ func (git *repoSync) SetupGitSSH(setupKnownHosts bool, pathsToSSHSecrets []strin
sshCmd += " -o StrictHostKeyChecking=no"
}
git.log.V(9).Info("setting GIT_SSH_COMMAND", "value", sshCmd)
git.log.V(9).Info("setting $GIT_SSH_COMMAND", "value", sshCmd)
if err := os.Setenv("GIT_SSH_COMMAND", sshCmd); err != nil {
return fmt.Errorf("can't set $GIT_SSH_COMMAND: %w", err)
}
@ -2151,7 +2173,8 @@ OPTIONS
Many options can be specified as either a commandline flag or an environment
variable, but flags are preferred because a misspelled flag is a fatal
error while a misspelled environment variable is silently ignored.
error while a misspelled environment variable is silently ignored. Some
options can only be specified as an environment variable.
--add-user, $GITSYNC_ADD_USER
Add a record to /etc/passwd for the current UID/GID. This is
@ -2169,11 +2192,12 @@ OPTIONS
--credential <string>, $GITSYNC_CREDENTIAL
Make one or more credentials available for authentication (see git
help credential). This is similar to --username and --password or
--password-file, but for specific URLs, for example when using
submodules. The value for this flag is either a JSON-encoded
object (see the schema below) or a JSON-encoded list of that same
object type. This flag may be specified more than once.
help credential). This is similar to --username and
$GITSYNC_PASSWORD or --password-file, but for specific URLs, for
example when using submodules. The value for this flag is either a
JSON-encoded object (see the schema below) or a JSON-encoded list
of that same object type. This flag may be specified more than
once.
Object schema:
- url: string, required
@ -2302,16 +2326,14 @@ OPTIONS
--one-time, $GITSYNC_ONE_TIME
Exit after one sync.
--password <string>, $GITSYNC_PASSWORD
$GITSYNC_PASSWORD
The password or personal access token (see github docs) to use for
git authentication (see --username). NOTE: for security reasons,
users should prefer --password-file or $GITSYNC_PASSWORD_FILE for
specifying the password.
git authentication (see --username). See also --password-file.
--password-file <string>, $GITSYNC_PASSWORD_FILE
The file from which the password or personal access token (see
github docs) to use for git authentication (see --username) will be
read.
read. See also $GITSYNC_PASSWORD.
--period <duration>, $GITSYNC_PERIOD
How long to wait between sync attempts. This must be at least
@ -2384,8 +2406,8 @@ OPTIONS
--username <string>, $GITSYNC_USERNAME
The username to use for git authentication (see --password-file or
--password). If more than one username and password is required
(e.g. with submodules), use --credential.
$GITSYNC_PASSWORD). If more than one username and password is
required (e.g. with submodules), use --credential.
-v, --verbose <int>, $GITSYNC_VERBOSE
Set the log verbosity level. Logs at this level and lower will be
@ -2443,31 +2465,31 @@ AUTHENTICATION
and "git@example.com:repo" will try to use SSH.
username/password
The --username (GITSYNC_USERNAME) and --password-file
(GITSYNC_PASSWORD_FILE) or --password (GITSYNC_PASSWORD) flags
will be used. To prevent password leaks, the --password-file flag
or GITSYNC_PASSWORD environment variable is almost always
preferred to the --password flag.
The --username ($GITSYNC_USERNAME) and $GITSYNC_PASSWORD or
--password-file ($GITSYNC_PASSWORD_FILE) flags will be used. To
prevent password leaks, the --password-file flag or
$GITSYNC_PASSWORD environment variable is almost always preferred
to the --password flag, which is deprecated.
A variant of this is --askpass-url (GITSYNC_ASKPASS_URL), which
A variant of this is --askpass-url ($GITSYNC_ASKPASS_URL), which
consults a URL (e.g. http://metadata) to get credentials on each
sync.
When using submodules it may be necessary to specify more than one
username and password, which can be done with --credential
(GITSYNC_CREDENTIAL). All of the username+password pairs, from
both --username/--password and --credential are fed into 'git
credential approve'.
($GITSYNC_CREDENTIAL). All of the username+password pairs, from
both --username/$GITSYNC_PASSWORD and --credential are fed into
'git credential approve'.
SSH
When an SSH transport is specified, the key(s) defined in
--ssh-key-file (GITSYNC_SSH_KEY_FILE) will be used. Users are
--ssh-key-file ($GITSYNC_SSH_KEY_FILE) will be used. Users are
strongly advised to also use --ssh-known-hosts
(GITSYNC_SSH_KNOWN_HOSTS) and --ssh-known-hosts-file
(GITSYNC_SSH_KNOWN_HOSTS_FILE) when using SSH.
($GITSYNC_SSH_KNOWN_HOSTS) and --ssh-known-hosts-file
($GITSYNC_SSH_KNOWN_HOSTS_FILE) when using SSH.
cookies
When --cookie-file (GITSYNC_COOKIE_FILE) is specified, the
When --cookie-file ($GITSYNC_COOKIE_FILE) is specified, the
associated cookies can contain authentication information.
HOOKS

22
test_e2e.sh
View File

@ -1784,7 +1784,7 @@ function e2e::auth_http_password() {
--root="$ROOT" \
--link="link" \
--username="wrong" \
--password="testpass"
--__env__GITSYNC_PASSWORD="testpass"
assert_file_absent "$ROOT/link/file"
# Try with wrong password
@ -1795,7 +1795,7 @@ function e2e::auth_http_password() {
--root="$ROOT" \
--link="link" \
--username="testuser" \
--password="wrong"
--__env__GITSYNC_PASSWORD="wrong"
assert_file_absent "$ROOT/link/file"
# Try with the right password
@ -1805,7 +1805,7 @@ function e2e::auth_http_password() {
--root="$ROOT" \
--link="link" \
--username="testuser" \
--password="testpass" \
--__env__GITSYNC_PASSWORD="testpass" \
assert_link_exists "$ROOT/link"
assert_file_exists "$ROOT/link/file"
@ -3528,6 +3528,14 @@ git config --global --add safe.directory '*'
# Store credentials for the test.
git config --global credential.helper "store --file $DIR/gitcreds"
# Log some info
if [[ -n "${VERBOSE:-}" ]]; then
git version
echo
docker version
echo
fi
FAILS=()
FINAL_RET=0
RUNS="${RUNS:-1}"
@ -3545,14 +3553,6 @@ if [[ -n "${VERBOSE:-}" ]]; then
fi
echo
# Log some info
if [[ -n "${VERBOSE:-}" ]]; then
git version
echo
docker version
echo
fi
# Iterate over the chosen tests and run them.
for t; do
TEST_FN="e2e::${t}"