From 0bc117ddc739af90da362f0aa8ef3e677e66d1ea Mon Sep 17 00:00:00 2001 From: Hidetake Iwata Date: Mon, 30 Sep 2019 18:27:23 +0900 Subject: [PATCH] Refactor (#158) * Refactor: template rendering * Refactor: rename DecodedIDToken fields * Refactor: expand command options * Refactor: improve help messages --- README.md | 10 ++--- docs/standalone-mode.md | 34 ++++++++--------- pkg/adaptors/cmd/cmd.go | 9 ----- pkg/adaptors/cmd/get_token.go | 11 ++++-- pkg/adaptors/cmd/root.go | 53 +++++++++++++-------------- pkg/adaptors/oidc/decoder.go | 12 +++--- pkg/adaptors/oidc/decoder_test.go | 6 +-- pkg/usecases/auth/auth.go | 12 +++--- pkg/usecases/auth/auth_test.go | 18 ++++----- pkg/usecases/standalone/standalone.go | 23 +++++++----- 10 files changed, 91 insertions(+), 97 deletions(-) diff --git a/README.md b/README.md index 4c718d4..850f32c 100644 --- a/README.md +++ b/README.md @@ -82,21 +82,21 @@ If you are looking for a specific version, see [the release tags](https://github Kubelogin supports the following options: ``` -% kubelogin get-token -h +% kubectl oidc-login get-token -h Run as a kubectl credential plugin Usage: kubelogin get-token [flags] Flags: - --listen-port ints Port to bind to the local server. If multiple ports are given, it will try the ports in order (default [8000,18000]) - --skip-open-browser If true, it does not open the browser on authentication - --username string If set, perform the resource owner password credentials grant - --password string If set, use the password instead of asking it --oidc-issuer-url string Issuer URL of the provider (mandatory) --oidc-client-id string Client ID of the provider (mandatory) --oidc-client-secret string Client secret of the provider --oidc-extra-scope strings Scopes to request to the provider + --listen-port ints Port to bind to the local server. If multiple ports are given, it will try the ports in order (default [8000,18000]) + --skip-open-browser If true, it does not open the browser on authentication + --username string If set, perform the resource owner password credentials grant + --password string If set, use the password instead of asking it --certificate-authority string Path to a cert file for the certificate authority --insecure-skip-tls-verify If true, the server's certificate will not be checked for validity. This will make your HTTPS connections insecure --token-cache-dir string Path to a directory for caching tokens (default "~/.kube/cache/oidc-login") diff --git a/docs/standalone-mode.md b/docs/standalone-mode.md index 3bd172e..544e9fc 100644 --- a/docs/standalone-mode.md +++ b/docs/standalone-mode.md @@ -78,38 +78,36 @@ If the refresh token has expired, kubelogin will proceed the authentication. Kubelogin supports the following options: ``` -% kubelogin -h -Login to the OpenID Connect provider and update the kubeconfig +% kubectl oidc-login -h +Login to the OpenID Connect provider. + +You need to set up the OIDC provider, role binding, Kubernetes API server and kubeconfig. +Run the following command to show the setup instruction: + + kubectl oidc-login setup + +See https://github.com/int128/kubelogin for more. Usage: - kubelogin [flags] - kubelogin [command] - -Examples: - # Login to the provider using the authorization code flow. - kubelogin - - # Login to the provider using the resource owner password credentials flow. - kubelogin --username USERNAME --password PASSWORD - - # Run as a credential plugin. - kubelogin get-token --oidc-issuer-url=https://issuer.example.com + main [flags] + main [command] Available Commands: get-token Run as a kubectl credential plugin help Help about any command + setup Show the setup instruction version Print the version information Flags: --kubeconfig string Path to the kubeconfig file --context string The name of the kubeconfig context to use --user string The name of the kubeconfig user to use. Prior to --context - --certificate-authority string Path to a cert file for the certificate authority - --insecure-skip-tls-verify If true, the server's certificate will not be checked for validity. This will make your HTTPS connections insecure --listen-port ints Port to bind to the local server. If multiple ports are given, it will try the ports in order (default [8000,18000]) --skip-open-browser If true, it does not open the browser on authentication --username string If set, perform the resource owner password credentials grant --password string If set, use the password instead of asking it + --certificate-authority string Path to a cert file for the certificate authority + --insecure-skip-tls-verify If true, the server's certificate will not be checked for validity. This will make your HTTPS connections insecure --add_dir_header If true, adds the file directory to the header --alsologtostderr log to standard error as well as files --log_backtrace_at traceLocation when logging hits line file:N, emit a stack trace (default :0) @@ -122,8 +120,8 @@ Flags: --stderrthreshold severity logs at or above this threshold go to stderr (default 2) -v, --v Level number for the log level verbosity --vmodule moduleSpec comma-separated list of pattern=N settings for file-filtered logging - -h, --help help for kubelogin - --version version for kubelogin + -h, --help help for main + --version version for main ``` ### Kubeconfig diff --git a/pkg/adaptors/cmd/cmd.go b/pkg/adaptors/cmd/cmd.go index 40ebe9e..7c70f0d 100644 --- a/pkg/adaptors/cmd/cmd.go +++ b/pkg/adaptors/cmd/cmd.go @@ -23,15 +23,6 @@ type Interface interface { Run(ctx context.Context, args []string, version string) int } -const examples = ` # Login to the provider using the authorization code flow. - %[1]s - - # Login to the provider using the resource owner password credentials flow. - %[1]s --username USERNAME --password PASSWORD - - # Run as a credential plugin. - %[1]s get-token --oidc-issuer-url=https://issuer.example.com` - var defaultListenPort = []int{8000, 18000} var defaultTokenCacheDir = homedir.HomeDir() + "/.kube/cache/oidc-login" diff --git a/pkg/adaptors/cmd/get_token.go b/pkg/adaptors/cmd/get_token.go index 6953488..3735e24 100644 --- a/pkg/adaptors/cmd/get_token.go +++ b/pkg/adaptors/cmd/get_token.go @@ -12,24 +12,29 @@ import ( // getTokenOptions represents the options for get-token command. type getTokenOptions struct { - loginOptions IssuerURL string ClientID string ClientSecret string ExtraScopes []string + ListenPort []int + SkipOpenBrowser bool + Username string + Password string CertificateAuthority string SkipTLSVerify bool - Verbose int TokenCacheDir string } func (o *getTokenOptions) register(f *pflag.FlagSet) { f.SortFlags = false - o.loginOptions.register(f) f.StringVar(&o.IssuerURL, "oidc-issuer-url", "", "Issuer URL of the provider (mandatory)") f.StringVar(&o.ClientID, "oidc-client-id", "", "Client ID of the provider (mandatory)") f.StringVar(&o.ClientSecret, "oidc-client-secret", "", "Client secret of the provider") f.StringSliceVar(&o.ExtraScopes, "oidc-extra-scope", nil, "Scopes to request to the provider") + f.IntSliceVar(&o.ListenPort, "listen-port", defaultListenPort, "Port to bind to the local server. If multiple ports are given, it will try the ports in order") + f.BoolVar(&o.SkipOpenBrowser, "skip-open-browser", false, "If true, it does not open the browser on authentication") + f.StringVar(&o.Username, "username", "", "If set, perform the resource owner password credentials grant") + f.StringVar(&o.Password, "password", "", "If set, use the password instead of asking it") f.StringVar(&o.CertificateAuthority, "certificate-authority", "", "Path to a cert file for the certificate authority") f.BoolVar(&o.SkipTLSVerify, "insecure-skip-tls-verify", false, "If true, the server's certificate will not be checked for validity. This will make your HTTPS connections insecure") f.StringVar(&o.TokenCacheDir, "token-cache-dir", defaultTokenCacheDir, "Path to a directory for caching tokens") diff --git a/pkg/adaptors/cmd/root.go b/pkg/adaptors/cmd/root.go index f4fe5d4..100552b 100644 --- a/pkg/adaptors/cmd/root.go +++ b/pkg/adaptors/cmd/root.go @@ -2,7 +2,6 @@ package cmd import ( "context" - "fmt" "github.com/int128/kubelogin/pkg/adaptors/kubeconfig" "github.com/int128/kubelogin/pkg/adaptors/logger" @@ -12,38 +11,40 @@ import ( "golang.org/x/xerrors" ) -// kubectlOptions represents kubectl specific options. -type kubectlOptions struct { +const longDescription = `Login to the OpenID Connect provider. + +You need to set up the OIDC provider, role binding, Kubernetes API server and kubeconfig. +Run the following command to show the setup instruction: + + kubectl oidc-login setup + +See https://github.com/int128/kubelogin for more. +` + +// rootOptions represents the options for the root command. +type rootOptions struct { Kubeconfig string Context string User string + ListenPort []int + SkipOpenBrowser bool + Username string + Password string CertificateAuthority string SkipTLSVerify bool } -func (o *kubectlOptions) register(f *pflag.FlagSet) { +func (o *rootOptions) register(f *pflag.FlagSet) { f.SortFlags = false f.StringVar(&o.Kubeconfig, "kubeconfig", "", "Path to the kubeconfig file") f.StringVar(&o.Context, "context", "", "The name of the kubeconfig context to use") f.StringVar(&o.User, "user", "", "The name of the kubeconfig user to use. Prior to --context") - f.StringVar(&o.CertificateAuthority, "certificate-authority", "", "Path to a cert file for the certificate authority") - f.BoolVar(&o.SkipTLSVerify, "insecure-skip-tls-verify", false, "If true, the server's certificate will not be checked for validity. This will make your HTTPS connections insecure") -} - -// loginOptions represents the options for Login use-case. -type loginOptions struct { - ListenPort []int - SkipOpenBrowser bool - Username string - Password string -} - -func (o *loginOptions) register(f *pflag.FlagSet) { - f.SortFlags = false f.IntSliceVar(&o.ListenPort, "listen-port", defaultListenPort, "Port to bind to the local server. If multiple ports are given, it will try the ports in order") f.BoolVar(&o.SkipOpenBrowser, "skip-open-browser", false, "If true, it does not open the browser on authentication") f.StringVar(&o.Username, "username", "", "If set, perform the resource owner password credentials grant") f.StringVar(&o.Password, "password", "", "If set, use the password instead of asking it") + f.StringVar(&o.CertificateAuthority, "certificate-authority", "", "Path to a cert file for the certificate authority") + f.BoolVar(&o.SkipTLSVerify, "insecure-skip-tls-verify", false, "If true, the server's certificate will not be checked for validity. This will make your HTTPS connections insecure") } type Root struct { @@ -52,15 +53,12 @@ type Root struct { } func (cmd *Root) New(ctx context.Context, executable string) *cobra.Command { - var o struct { - kubectlOptions - loginOptions - } + var o rootOptions rootCmd := &cobra.Command{ - Use: executable, - Short: "Login to the OpenID Connect provider and update the kubeconfig", - Example: fmt.Sprintf(examples, executable), - Args: cobra.NoArgs, + Use: executable, + Short: "Login to the OpenID Connect provider", + Long: longDescription, + Args: cobra.NoArgs, RunE: func(*cobra.Command, []string) error { in := standalone.Input{ KubeconfigFilename: o.Kubeconfig, @@ -79,8 +77,7 @@ func (cmd *Root) New(ctx context.Context, executable string) *cobra.Command { return nil }, } - o.kubectlOptions.register(rootCmd.Flags()) - o.loginOptions.register(rootCmd.Flags()) + o.register(rootCmd.Flags()) cmd.Logger.AddFlags(rootCmd.PersistentFlags()) return rootCmd } diff --git a/pkg/adaptors/oidc/decoder.go b/pkg/adaptors/oidc/decoder.go index 233b125..a4590e0 100644 --- a/pkg/adaptors/oidc/decoder.go +++ b/pkg/adaptors/oidc/decoder.go @@ -16,9 +16,9 @@ type DecoderInterface interface { } type DecodedIDToken struct { - IDTokenSubject string - IDTokenExpiry time.Time - IDTokenClaims map[string]string // string representation of claims for logging + Subject string + Expiry time.Time + Claims map[string]string // string representation of claims for logging } type Decoder struct{} @@ -43,9 +43,9 @@ func (d *Decoder) DecodeIDToken(t string) (*DecodedIDToken, error) { return nil, xerrors.Errorf("could not decode the json of token: %w", err) } return &DecodedIDToken{ - IDTokenSubject: claims.Subject, - IDTokenExpiry: time.Unix(claims.ExpiresAt, 0), - IDTokenClaims: dumpRawClaims(rawClaims), + Subject: claims.Subject, + Expiry: time.Unix(claims.ExpiresAt, 0), + Claims: dumpRawClaims(rawClaims), }, nil } diff --git a/pkg/adaptors/oidc/decoder_test.go b/pkg/adaptors/oidc/decoder_test.go index 58d2bb8..a2d7686 100644 --- a/pkg/adaptors/oidc/decoder_test.go +++ b/pkg/adaptors/oidc/decoder_test.go @@ -21,10 +21,10 @@ func TestDecoder_DecodeIDToken(t *testing.T) { if err != nil { t.Fatalf("DecodeIDToken error: %s", err) } - if decodedToken.IDTokenExpiry != expiry { - t.Errorf("IDTokenExpiry wants %s but %s", expiry, decodedToken.IDTokenExpiry) + if decodedToken.Expiry != expiry { + t.Errorf("Expiry wants %s but %s", expiry, decodedToken.Expiry) } - t.Logf("IDTokenClaims=%+v", decodedToken.IDTokenClaims) + t.Logf("Claims=%+v", decodedToken.Claims) }) t.Run("InvalidToken", func(t *testing.T) { decodedToken, err := decoder.DecodeIDToken("HEADER.INVALID_TOKEN.SIGNATURE") diff --git a/pkg/usecases/auth/auth.go b/pkg/usecases/auth/auth.go index 9e78815..1671d08 100644 --- a/pkg/usecases/auth/auth.go +++ b/pkg/usecases/auth/auth.go @@ -85,18 +85,18 @@ func (u *Authentication) Do(ctx context.Context, in Input) (*Output, error) { if err != nil { return nil, xerrors.Errorf("invalid token and you need to remove the cache: %w", err) } - if token.IDTokenExpiry.After(time.Now()) { //TODO: inject time service - u.Logger.V(1).Infof("you already have a valid token until %s", token.IDTokenExpiry) + if token.Expiry.After(time.Now()) { //TODO: inject time service + u.Logger.V(1).Infof("you already have a valid token until %s", token.Expiry) return &Output{ AlreadyHasValidIDToken: true, IDToken: in.OIDCConfig.IDToken, RefreshToken: in.OIDCConfig.RefreshToken, - IDTokenSubject: token.IDTokenSubject, - IDTokenExpiry: token.IDTokenExpiry, - IDTokenClaims: token.IDTokenClaims, + IDTokenSubject: token.Subject, + IDTokenExpiry: token.Expiry, + IDTokenClaims: token.Claims, }, nil } - u.Logger.V(1).Infof("you have an expired token at %s", token.IDTokenExpiry) + u.Logger.V(1).Infof("you have an expired token at %s", token.Expiry) } u.Logger.V(1).Infof("initializing an OIDCFactory client") diff --git a/pkg/usecases/auth/auth_test.go b/pkg/usecases/auth/auth_test.go index 778a8b7..2049986 100644 --- a/pkg/usecases/auth/auth_test.go +++ b/pkg/usecases/auth/auth_test.go @@ -274,9 +274,9 @@ func TestAuthentication_Do(t *testing.T) { mockOIDCDecoder.EXPECT(). DecodeIDToken("VALID_ID_TOKEN"). Return(&oidc.DecodedIDToken{ - IDTokenSubject: "YOUR_SUBJECT", - IDTokenExpiry: futureTime, - IDTokenClaims: dummyTokenClaims, + Subject: "YOUR_SUBJECT", + Expiry: futureTime, + Claims: dummyTokenClaims, }, nil) u := Authentication{ OIDCFactory: mock_oidc.NewMockFactoryInterface(ctrl), @@ -315,9 +315,9 @@ func TestAuthentication_Do(t *testing.T) { mockOIDCDecoder.EXPECT(). DecodeIDToken("EXPIRED_ID_TOKEN"). Return(&oidc.DecodedIDToken{ - IDTokenSubject: "YOUR_SUBJECT", - IDTokenExpiry: pastTime, - IDTokenClaims: dummyTokenClaims, + Subject: "YOUR_SUBJECT", + Expiry: pastTime, + Claims: dummyTokenClaims, }, nil) mockOIDCClient := mock_oidc.NewMockInterface(ctrl) mockOIDCClient.EXPECT(). @@ -373,9 +373,9 @@ func TestAuthentication_Do(t *testing.T) { mockOIDCDecoder.EXPECT(). DecodeIDToken("EXPIRED_ID_TOKEN"). Return(&oidc.DecodedIDToken{ - IDTokenSubject: "YOUR_SUBJECT", - IDTokenExpiry: pastTime, - IDTokenClaims: dummyTokenClaims, + Subject: "YOUR_SUBJECT", + Expiry: pastTime, + Claims: dummyTokenClaims, }, nil) mockOIDCClient := mock_oidc.NewMockInterface(ctrl) mockOIDCClient.EXPECT(). diff --git a/pkg/usecases/standalone/standalone.go b/pkg/usecases/standalone/standalone.go index 251c3e2..2121a1d 100644 --- a/pkg/usecases/standalone/standalone.go +++ b/pkg/usecases/standalone/standalone.go @@ -37,10 +37,8 @@ type Input struct { SkipTLSVerify bool } -const oidcConfigErrorMessage = `NOTE: -You need to setup the kubeconfig for OpenID Connect authentication. +const oidcConfigErrorMessage = `You need to set up the kubeconfig for OpenID Connect authentication. See https://github.com/int128/kubelogin for more. - ` // Standalone provides the use case of explicit login. @@ -99,13 +97,13 @@ func (u *Standalone) Do(ctx context.Context, in Input) error { return nil } -var deprecation = template.Must(template.New("").Parse( +var deprecationTpl = template.Must(template.New("").Parse( `IMPORTANT NOTICE: The credential plugin mode is available since v1.14.0. Kubectl will automatically run kubelogin and you do not need to run kubelogin explicitly. You can switch to the credential plugin mode by setting the following user to -{{ .kubeconfig }}. +{{ .Kubeconfig }}. --- users: - name: oidc @@ -116,7 +114,7 @@ users: args: - oidc-login - get-token -{{- range .args }} +{{- range .Args }} - {{ . }} {{- end }} --- @@ -124,6 +122,11 @@ See https://github.com/int128/kubelogin for more. `)) +type deprecationVars struct { + Kubeconfig string + Args []string +} + func (u *Standalone) showDeprecation(in Input, p *kubeconfig.AuthProvider) error { var args []string args = append(args, "--oidc-issuer-url="+p.OIDCConfig.IDPIssuerURL) @@ -144,12 +147,12 @@ func (u *Standalone) showDeprecation(in Input, p *kubeconfig.AuthProvider) error args = append(args, "--username="+in.Username) } - m := map[string]interface{}{ - "kubeconfig": p.LocationOfOrigin, - "args": args, + v := deprecationVars{ + Kubeconfig: p.LocationOfOrigin, + Args: args, } var b strings.Builder - if err := deprecation.ExecuteTemplate(&b, "", m); err != nil { + if err := deprecationTpl.Execute(&b, &v); err != nil { return xerrors.Errorf("could not render the template: %w", err) } u.Logger.Printf("%s", b.String())