From 7196c64bec4c64a24daebb08e268c066dab71690 Mon Sep 17 00:00:00 2001 From: Hidetake Iwata Date: Fri, 24 Jul 2020 20:53:31 +0900 Subject: [PATCH 1/5] Refactor: rename to addFlags() --- pkg/adaptors/cmd/get_token.go | 14 +++++++------- pkg/adaptors/cmd/root.go | 16 ++++++++-------- pkg/adaptors/cmd/setup.go | 8 ++++---- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/pkg/adaptors/cmd/get_token.go b/pkg/adaptors/cmd/get_token.go index f7eb550d..4f4e1596 100644 --- a/pkg/adaptors/cmd/get_token.go +++ b/pkg/adaptors/cmd/get_token.go @@ -21,17 +21,16 @@ type getTokenOptions struct { authenticationOptions authenticationOptions } -func (o *getTokenOptions) register(f *pflag.FlagSet) { - f.SortFlags = false +func (o *getTokenOptions) addFlags(f *pflag.FlagSet) { 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.StringVar(&o.CACertFilename, "certificate-authority", "", "Path to a cert file for the certificate authority") - f.StringVar(&o.CACertData, "certificate-authority-data", "", "Base64 encoded data 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") - o.authenticationOptions.register(f) + f.StringVar(&o.CACertData, "certificate-authority-data", "", "Base64 encoded cert for the certificate authority") + f.BoolVar(&o.SkipTLSVerify, "insecure-skip-tls-verify", false, "If set, 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 token cache") + o.authenticationOptions.addFlags(f) } type GetToken struct { @@ -78,6 +77,7 @@ func (cmd *GetToken) New() *cobra.Command { return nil }, } - o.register(c.Flags()) + c.Flags().SortFlags = false + o.addFlags(c.Flags()) return c } diff --git a/pkg/adaptors/cmd/root.go b/pkg/adaptors/cmd/root.go index 6468974d..9614437e 100644 --- a/pkg/adaptors/cmd/root.go +++ b/pkg/adaptors/cmd/root.go @@ -33,14 +33,13 @@ type rootOptions struct { authenticationOptions authenticationOptions } -func (o *rootOptions) register(f *pflag.FlagSet) { - f.SortFlags = false +func (o *rootOptions) addFlags(f *pflag.FlagSet) { 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") - o.authenticationOptions.register(f) + o.authenticationOptions.addFlags(f) } type authenticationOptions struct { @@ -76,7 +75,7 @@ var allGrantType = strings.Join([]string{ "password", }, "|") -func (o *authenticationOptions) register(f *pflag.FlagSet) { +func (o *authenticationOptions) addFlags(f *pflag.FlagSet) { f.StringVar(&o.GrantType, "grant-type", "auto", fmt.Sprintf("The authorization grant type to use. One of (%s)", allGrantType)) f.StringSliceVar(&o.ListenAddress, "listen-address", defaultListenAddress, "Address to bind to the local server. If multiple addresses are given, it will try binding in order") //TODO: remove the deprecated flag @@ -119,7 +118,7 @@ type Root struct { func (cmd *Root) New() *cobra.Command { var o rootOptions - rootCmd := &cobra.Command{ + c := &cobra.Command{ Use: "kubelogin", Short: "Login to the OpenID Connect provider", Long: longDescription, @@ -143,7 +142,8 @@ func (cmd *Root) New() *cobra.Command { return nil }, } - o.register(rootCmd.Flags()) - cmd.Logger.AddFlags(rootCmd.PersistentFlags()) - return rootCmd + c.Flags().SortFlags = false + o.addFlags(c.Flags()) + cmd.Logger.AddFlags(c.PersistentFlags()) + return c } diff --git a/pkg/adaptors/cmd/setup.go b/pkg/adaptors/cmd/setup.go index 5d0373de..c12fb80a 100644 --- a/pkg/adaptors/cmd/setup.go +++ b/pkg/adaptors/cmd/setup.go @@ -19,8 +19,7 @@ type setupOptions struct { authenticationOptions authenticationOptions } -func (o *setupOptions) register(f *pflag.FlagSet) { - f.SortFlags = false +func (o *setupOptions) addFlags(f *pflag.FlagSet) { f.StringVar(&o.IssuerURL, "oidc-issuer-url", "", "Issuer URL of the provider") f.StringVar(&o.ClientID, "oidc-client-id", "", "Client ID of the provider") f.StringVar(&o.ClientSecret, "oidc-client-secret", "", "Client secret of the provider") @@ -28,7 +27,7 @@ func (o *setupOptions) register(f *pflag.FlagSet) { f.StringVar(&o.CACertFilename, "certificate-authority", "", "Path to a cert file for the certificate authority") f.StringVar(&o.CACertData, "certificate-authority-data", "", "Base64 encoded data 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") - o.authenticationOptions.register(f) + o.authenticationOptions.addFlags(f) } type Setup struct { @@ -69,6 +68,7 @@ func (cmd *Setup) New() *cobra.Command { return nil }, } - o.register(c.Flags()) + c.Flags().SortFlags = false + o.addFlags(c.Flags()) return c } From 8197b5b35aa6c6178f477c594a051d93c547e958 Mon Sep 17 00:00:00 2001 From: Hidetake Iwata Date: Fri, 24 Jul 2020 21:00:00 +0900 Subject: [PATCH 2/5] Refactor: extract authentication.go --- pkg/adaptors/cmd/authentication.go | 79 ++++++++++++++++++++++++++++++ pkg/adaptors/cmd/root.go | 73 --------------------------- 2 files changed, 79 insertions(+), 73 deletions(-) create mode 100644 pkg/adaptors/cmd/authentication.go diff --git a/pkg/adaptors/cmd/authentication.go b/pkg/adaptors/cmd/authentication.go new file mode 100644 index 00000000..7f98ea1f --- /dev/null +++ b/pkg/adaptors/cmd/authentication.go @@ -0,0 +1,79 @@ +package cmd + +import ( + "fmt" + "strings" + + "github.com/int128/kubelogin/pkg/usecases/authentication" + "github.com/spf13/pflag" + "golang.org/x/xerrors" +) + +type authenticationOptions struct { + GrantType string + ListenAddress []string + ListenPort []int // deprecated + SkipOpenBrowser bool + RedirectURLHostname string + AuthRequestExtraParams map[string]string + Username string + Password string +} + +// determineListenAddress returns the addresses from the flags. +// Note that --listen-address is always given due to the default value. +// If --listen-port is not set, it returns --listen-address. +// If --listen-port is set, it returns the strings of --listen-port. +func (o *authenticationOptions) determineListenAddress() []string { + if len(o.ListenPort) == 0 { + return o.ListenAddress + } + var a []string + for _, p := range o.ListenPort { + a = append(a, fmt.Sprintf("127.0.0.1:%d", p)) + } + return a +} + +var allGrantType = strings.Join([]string{ + "auto", + "authcode", + "authcode-keyboard", + "password", +}, "|") + +func (o *authenticationOptions) addFlags(f *pflag.FlagSet) { + f.StringVar(&o.GrantType, "grant-type", "auto", fmt.Sprintf("The authorization grant type to use. One of (%s)", allGrantType)) + f.StringSliceVar(&o.ListenAddress, "listen-address", defaultListenAddress, "Address to bind to the local server. If multiple addresses are given, it will try binding in order") + //TODO: remove the deprecated flag + f.IntSliceVar(&o.ListenPort, "listen-port", nil, "(Deprecated: use --listen-address)") + f.BoolVar(&o.SkipOpenBrowser, "skip-open-browser", false, "If true, it does not open the browser on authentication") + f.StringVar(&o.RedirectURLHostname, "oidc-redirect-url-hostname", "localhost", "Hostname of the redirect URL") + f.StringToStringVar(&o.AuthRequestExtraParams, "oidc-auth-request-extra-params", nil, "Extra query parameters to send with an authentication request") + 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") +} + +func (o *authenticationOptions) grantOptionSet() (s authentication.GrantOptionSet, err error) { + switch { + case o.GrantType == "authcode" || (o.GrantType == "auto" && o.Username == ""): + s.AuthCodeOption = &authentication.AuthCodeOption{ + BindAddress: o.determineListenAddress(), + SkipOpenBrowser: o.SkipOpenBrowser, + RedirectURLHostname: o.RedirectURLHostname, + AuthRequestExtraParams: o.AuthRequestExtraParams, + } + case o.GrantType == "authcode-keyboard": + s.AuthCodeKeyboardOption = &authentication.AuthCodeKeyboardOption{ + AuthRequestExtraParams: o.AuthRequestExtraParams, + } + case o.GrantType == "password" || (o.GrantType == "auto" && o.Username != ""): + s.ROPCOption = &authentication.ROPCOption{ + Username: o.Username, + Password: o.Password, + } + default: + err = xerrors.Errorf("grant-type must be one of (%s)", allGrantType) + } + return +} diff --git a/pkg/adaptors/cmd/root.go b/pkg/adaptors/cmd/root.go index 9614437e..e40ed86e 100644 --- a/pkg/adaptors/cmd/root.go +++ b/pkg/adaptors/cmd/root.go @@ -1,12 +1,8 @@ package cmd import ( - "fmt" - "strings" - "github.com/int128/kubelogin/pkg/adaptors/kubeconfig" "github.com/int128/kubelogin/pkg/adaptors/logger" - "github.com/int128/kubelogin/pkg/usecases/authentication" "github.com/int128/kubelogin/pkg/usecases/standalone" "github.com/spf13/cobra" "github.com/spf13/pflag" @@ -42,75 +38,6 @@ func (o *rootOptions) addFlags(f *pflag.FlagSet) { o.authenticationOptions.addFlags(f) } -type authenticationOptions struct { - GrantType string - ListenAddress []string - ListenPort []int // deprecated - SkipOpenBrowser bool - RedirectURLHostname string - AuthRequestExtraParams map[string]string - Username string - Password string -} - -// determineListenAddress returns the addresses from the flags. -// Note that --listen-address is always given due to the default value. -// If --listen-port is not set, it returns --listen-address. -// If --listen-port is set, it returns the strings of --listen-port. -func (o *authenticationOptions) determineListenAddress() []string { - if len(o.ListenPort) == 0 { - return o.ListenAddress - } - var a []string - for _, p := range o.ListenPort { - a = append(a, fmt.Sprintf("127.0.0.1:%d", p)) - } - return a -} - -var allGrantType = strings.Join([]string{ - "auto", - "authcode", - "authcode-keyboard", - "password", -}, "|") - -func (o *authenticationOptions) addFlags(f *pflag.FlagSet) { - f.StringVar(&o.GrantType, "grant-type", "auto", fmt.Sprintf("The authorization grant type to use. One of (%s)", allGrantType)) - f.StringSliceVar(&o.ListenAddress, "listen-address", defaultListenAddress, "Address to bind to the local server. If multiple addresses are given, it will try binding in order") - //TODO: remove the deprecated flag - f.IntSliceVar(&o.ListenPort, "listen-port", nil, "(Deprecated: use --listen-address)") - f.BoolVar(&o.SkipOpenBrowser, "skip-open-browser", false, "If true, it does not open the browser on authentication") - f.StringVar(&o.RedirectURLHostname, "oidc-redirect-url-hostname", "localhost", "Hostname of the redirect URL") - f.StringToStringVar(&o.AuthRequestExtraParams, "oidc-auth-request-extra-params", nil, "Extra query parameters to send with an authentication request") - 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") -} - -func (o *authenticationOptions) grantOptionSet() (s authentication.GrantOptionSet, err error) { - switch { - case o.GrantType == "authcode" || (o.GrantType == "auto" && o.Username == ""): - s.AuthCodeOption = &authentication.AuthCodeOption{ - BindAddress: o.determineListenAddress(), - SkipOpenBrowser: o.SkipOpenBrowser, - RedirectURLHostname: o.RedirectURLHostname, - AuthRequestExtraParams: o.AuthRequestExtraParams, - } - case o.GrantType == "authcode-keyboard": - s.AuthCodeKeyboardOption = &authentication.AuthCodeKeyboardOption{ - AuthRequestExtraParams: o.AuthRequestExtraParams, - } - case o.GrantType == "password" || (o.GrantType == "auto" && o.Username != ""): - s.ROPCOption = &authentication.ROPCOption{ - Username: o.Username, - Password: o.Password, - } - default: - err = xerrors.Errorf("grant-type must be one of (%s)", allGrantType) - } - return -} - type Root struct { Standalone standalone.Interface Logger logger.Interface From 1ae2008e28b9496e718ad054e2485c91e9a46e79 Mon Sep 17 00:00:00 2001 From: Hidetake Iwata Date: Sun, 26 Jul 2020 08:43:32 +0900 Subject: [PATCH 3/5] Refactor: extract tlsOptions --- pkg/adaptors/cmd/cmd_test.go | 2 ++ pkg/adaptors/cmd/get_token.go | 14 +++++--------- pkg/adaptors/cmd/root.go | 11 +++++------ pkg/adaptors/cmd/setup.go | 14 +++++--------- pkg/adaptors/cmd/tls.go | 15 +++++++++++++++ pkg/usecases/standalone/standalone.go | 10 ++++++++-- pkg/usecases/standalone/standalone_test.go | 9 ++++++--- 7 files changed, 46 insertions(+), 29 deletions(-) create mode 100644 pkg/adaptors/cmd/tls.go diff --git a/pkg/adaptors/cmd/cmd_test.go b/pkg/adaptors/cmd/cmd_test.go index 77ff4338..912fa62e 100644 --- a/pkg/adaptors/cmd/cmd_test.go +++ b/pkg/adaptors/cmd/cmd_test.go @@ -71,6 +71,7 @@ func TestCmd_Run(t *testing.T) { "--context", "hello.k8s.local", "--user", "google", "--certificate-authority", "/path/to/cacert", + "--certificate-authority-data", "BASE64ENCODED", "--insecure-skip-tls-verify", "-v1", "--grant-type", "authcode", @@ -85,6 +86,7 @@ func TestCmd_Run(t *testing.T) { KubeconfigContext: "hello.k8s.local", KubeconfigUser: "google", CACertFilename: "/path/to/cacert", + CACertData: "BASE64ENCODED", SkipTLSVerify: true, GrantOptionSet: authentication.GrantOptionSet{ AuthCodeOption: &authentication.AuthCodeOption{ diff --git a/pkg/adaptors/cmd/get_token.go b/pkg/adaptors/cmd/get_token.go index 4f4e1596..356e0d66 100644 --- a/pkg/adaptors/cmd/get_token.go +++ b/pkg/adaptors/cmd/get_token.go @@ -14,10 +14,8 @@ type getTokenOptions struct { ClientID string ClientSecret string ExtraScopes []string - CACertFilename string - CACertData string - SkipTLSVerify bool TokenCacheDir string + tlsOptions tlsOptions authenticationOptions authenticationOptions } @@ -26,10 +24,8 @@ func (o *getTokenOptions) addFlags(f *pflag.FlagSet) { 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.StringVar(&o.CACertFilename, "certificate-authority", "", "Path to a cert file for the certificate authority") - f.StringVar(&o.CACertData, "certificate-authority-data", "", "Base64 encoded cert for the certificate authority") - f.BoolVar(&o.SkipTLSVerify, "insecure-skip-tls-verify", false, "If set, 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 token cache") + o.tlsOptions.addFlags(f) o.authenticationOptions.addFlags(f) } @@ -65,9 +61,9 @@ func (cmd *GetToken) New() *cobra.Command { ClientID: o.ClientID, ClientSecret: o.ClientSecret, ExtraScopes: o.ExtraScopes, - CACertFilename: o.CACertFilename, - CACertData: o.CACertData, - SkipTLSVerify: o.SkipTLSVerify, + CACertFilename: o.tlsOptions.CACertFilename, + CACertData: o.tlsOptions.CACertData, + SkipTLSVerify: o.tlsOptions.SkipTLSVerify, TokenCacheDir: o.TokenCacheDir, GrantOptionSet: grantOptionSet, } diff --git a/pkg/adaptors/cmd/root.go b/pkg/adaptors/cmd/root.go index e40ed86e..44f6c4a5 100644 --- a/pkg/adaptors/cmd/root.go +++ b/pkg/adaptors/cmd/root.go @@ -24,8 +24,7 @@ type rootOptions struct { Kubeconfig string Context string User string - CertificateAuthority string - SkipTLSVerify bool + tlsOptions tlsOptions authenticationOptions authenticationOptions } @@ -33,8 +32,7 @@ func (o *rootOptions) addFlags(f *pflag.FlagSet) { 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") + o.tlsOptions.addFlags(f) o.authenticationOptions.addFlags(f) } @@ -59,8 +57,9 @@ func (cmd *Root) New() *cobra.Command { KubeconfigFilename: o.Kubeconfig, KubeconfigContext: kubeconfig.ContextName(o.Context), KubeconfigUser: kubeconfig.UserName(o.User), - CACertFilename: o.CertificateAuthority, - SkipTLSVerify: o.SkipTLSVerify, + CACertFilename: o.tlsOptions.CACertFilename, + CACertData: o.tlsOptions.CACertData, + SkipTLSVerify: o.tlsOptions.SkipTLSVerify, GrantOptionSet: grantOptionSet, } if err := cmd.Standalone.Do(c.Context(), in); err != nil { diff --git a/pkg/adaptors/cmd/setup.go b/pkg/adaptors/cmd/setup.go index c12fb80a..3ca41182 100644 --- a/pkg/adaptors/cmd/setup.go +++ b/pkg/adaptors/cmd/setup.go @@ -13,9 +13,7 @@ type setupOptions struct { ClientID string ClientSecret string ExtraScopes []string - CACertFilename string - CACertData string - SkipTLSVerify bool + tlsOptions tlsOptions authenticationOptions authenticationOptions } @@ -24,9 +22,7 @@ func (o *setupOptions) addFlags(f *pflag.FlagSet) { f.StringVar(&o.ClientID, "oidc-client-id", "", "Client ID of the provider") 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.StringVar(&o.CACertFilename, "certificate-authority", "", "Path to a cert file for the certificate authority") - f.StringVar(&o.CACertData, "certificate-authority-data", "", "Base64 encoded data 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") + o.tlsOptions.addFlags(f) o.authenticationOptions.addFlags(f) } @@ -50,9 +46,9 @@ func (cmd *Setup) New() *cobra.Command { ClientID: o.ClientID, ClientSecret: o.ClientSecret, ExtraScopes: o.ExtraScopes, - CACertFilename: o.CACertFilename, - CACertData: o.CACertData, - SkipTLSVerify: o.SkipTLSVerify, + CACertFilename: o.tlsOptions.CACertFilename, + CACertData: o.tlsOptions.CACertData, + SkipTLSVerify: o.tlsOptions.SkipTLSVerify, GrantOptionSet: grantOptionSet, } if c.Flags().Lookup("listen-address").Changed { diff --git a/pkg/adaptors/cmd/tls.go b/pkg/adaptors/cmd/tls.go new file mode 100644 index 00000000..9a56ac1a --- /dev/null +++ b/pkg/adaptors/cmd/tls.go @@ -0,0 +1,15 @@ +package cmd + +import "github.com/spf13/pflag" + +type tlsOptions struct { + CACertFilename string + CACertData string + SkipTLSVerify bool +} + +func (o *tlsOptions) addFlags(f *pflag.FlagSet) { + f.StringVar(&o.CACertFilename, "certificate-authority", "", "Path to a cert file for the certificate authority") + f.StringVar(&o.CACertData, "certificate-authority-data", "", "Base64 encoded cert for the certificate authority") + f.BoolVar(&o.SkipTLSVerify, "insecure-skip-tls-verify", false, "If set, the server's certificate will not be checked for validity. This will make your HTTPS connections insecure") +} diff --git a/pkg/usecases/standalone/standalone.go b/pkg/usecases/standalone/standalone.go index 8773e1cc..4acbb488 100644 --- a/pkg/usecases/standalone/standalone.go +++ b/pkg/usecases/standalone/standalone.go @@ -30,7 +30,8 @@ type Input struct { KubeconfigFilename string // Default to the environment variable or global config as kubectl KubeconfigContext kubeconfig.ContextName // Default to the current context but ignored if KubeconfigUser is set KubeconfigUser kubeconfig.UserName // Default to the user of the context - CACertFilename string // If set, use the CA cert + CACertFilename string // optional + CACertData string // optional SkipTLSVerify bool GrantOptionSet authentication.GrantOptionSet } @@ -78,7 +79,12 @@ func (u *Standalone) Do(ctx context.Context, in Input) error { } if in.CACertFilename != "" { if err := certPool.AddFile(in.CACertFilename); err != nil { - return xerrors.Errorf("could not load the certificate: %w", err) + return xerrors.Errorf("could not load the certificate file: %w", err) + } + } + if in.CACertData != "" { + if err := certPool.AddBase64Encoded(in.CACertData); err != nil { + return xerrors.Errorf("could not load the certificate data: %w", err) } } out, err := u.Authentication.Do(ctx, authentication.Input{ diff --git a/pkg/usecases/standalone/standalone_test.go b/pkg/usecases/standalone/standalone_test.go index 4a049a72..7e93b342 100644 --- a/pkg/usecases/standalone/standalone_test.go +++ b/pkg/usecases/standalone/standalone_test.go @@ -34,6 +34,7 @@ func TestStandalone_Do(t *testing.T) { KubeconfigContext: "theContext", KubeconfigUser: "theUser", CACertFilename: "/path/to/cert1", + CACertData: "BASE64ENCODED1", SkipTLSVerify: true, GrantOptionSet: grantOptionSet, } @@ -44,7 +45,7 @@ func TestStandalone_Do(t *testing.T) { ClientID: "YOUR_CLIENT_ID", ClientSecret: "YOUR_CLIENT_SECRET", IDPCertificateAuthority: "/path/to/cert2", - IDPCertificateAuthorityData: "BASE64ENCODED", + IDPCertificateAuthorityData: "BASE64ENCODED2", } mockCertPool := mock_certpool.NewMockInterface(ctrl) mockCertPool.EXPECT(). @@ -52,7 +53,9 @@ func TestStandalone_Do(t *testing.T) { mockCertPool.EXPECT(). AddFile("/path/to/cert2") mockCertPool.EXPECT(). - AddBase64Encoded("BASE64ENCODED") + AddBase64Encoded("BASE64ENCODED1") + mockCertPool.EXPECT(). + AddBase64Encoded("BASE64ENCODED2") mockKubeconfig := mock_kubeconfig.NewMockInterface(ctrl) mockKubeconfig.EXPECT(). GetCurrentAuthProvider("/path/to/kubeconfig", kubeconfig.ContextName("theContext"), kubeconfig.UserName("theUser")). @@ -65,7 +68,7 @@ func TestStandalone_Do(t *testing.T) { ClientID: "YOUR_CLIENT_ID", ClientSecret: "YOUR_CLIENT_SECRET", IDPCertificateAuthority: "/path/to/cert2", - IDPCertificateAuthorityData: "BASE64ENCODED", + IDPCertificateAuthorityData: "BASE64ENCODED2", IDToken: "YOUR_ID_TOKEN", RefreshToken: "YOUR_REFRESH_TOKEN", }) From 98b84d87e05a3bd8b4b7a8ee1140838ce4f82bc5 Mon Sep 17 00:00:00 2001 From: Hidetake Iwata Date: Sun, 26 Jul 2020 11:19:38 +0900 Subject: [PATCH 4/5] Refactor: change options description --- README.md | 21 ++++++------ docs/standalone-mode.md | 53 ------------------------------ pkg/adaptors/cmd/authentication.go | 19 ++++++----- pkg/adaptors/cmd/root.go | 12 +++---- 4 files changed, 27 insertions(+), 78 deletions(-) diff --git a/README.md b/README.md index 4003e0ca..339f2710 100644 --- a/README.md +++ b/README.md @@ -120,18 +120,17 @@ Flags: --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 + --token-cache-dir string Path to a directory for token cache (default "~/.kube/cache/oidc-login") --certificate-authority string Path to a cert file for the certificate authority - --certificate-authority-data string Base64 encoded data 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") - --grant-type string The authorization grant type to use. One of (auto|authcode|authcode-keyboard|password) (default "auto") - --listen-address strings Address to bind to the local server. If multiple addresses are given, it will try binding in order (default [127.0.0.1:8000,127.0.0.1:18000]) - --listen-port ints (Deprecated: use --listen-address) - --skip-open-browser If true, it does not open the browser on authentication - --oidc-redirect-url-hostname string Hostname of the redirect URL (default "localhost") - --oidc-auth-request-extra-params stringToString Extra query parameters to send with an authentication request (default []) - --username string If set, perform the resource owner password credentials grant - --password string If set, use the password instead of asking it + --certificate-authority-data string Base64 encoded cert for the certificate authority + --insecure-skip-tls-verify If set, the server's certificate will not be checked for validity. This will make your HTTPS connections insecure + --grant-type string Authorization grant type to use. One of (auto|authcode|authcode-keyboard|password) (default "auto") + --listen-address strings [authcode] Address to bind to the local server. If multiple addresses are set, it will try binding in order (default [127.0.0.1:8000,127.0.0.1:18000]) + --skip-open-browser [authcode] Do not open the browser automatically + --oidc-redirect-url-hostname string [authcode] Hostname of the redirect URL (default "localhost") + --oidc-auth-request-extra-params stringToString [authcode, authcode-keyboard] Extra query parameters to send with an authentication request (default []) + --username string [password] Username for resource owner password credentials grant + --password string [password] Password for resource owner password credentials grant -h, --help help for get-token Global Flags: diff --git a/docs/standalone-mode.md b/docs/standalone-mode.md index e3fbf34b..195bf4f7 100644 --- a/docs/standalone-mode.md +++ b/docs/standalone-mode.md @@ -75,59 +75,6 @@ If the refresh token has expired, kubelogin will proceed the authentication. ## Usage -Kubelogin supports the following options: - -``` -% 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: - 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 - --grant-type string The authorization grant type to use. One of (auto|authcode|authcode-keyboard|password) (default "auto") - --listen-address strings Address to bind to the local server. If multiple addresses are given, it will try binding in order (default [127.0.0.1:8000,127.0.0.1:18000]) - --listen-port ints (Deprecated: use --listen-address) - --skip-open-browser If true, it does not open the browser on authentication - --oidc-redirect-url-hostname string Hostname of the redirect URL (default "localhost") - --oidc-auth-request-extra-params stringToString Extra query parameters to send with an authentication request (default []) - --username string If set, perform the resource owner password credentials grant - --password string If set, use the password instead of asking it - --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) - --log_dir string If non-empty, write log files in this directory - --log_file string If non-empty, use this log file - --log_file_max_size uint Defines the maximum size a log file can grow to. Unit is megabytes. If the value is 0, the maximum file size is unlimited. (default 1800) - --logtostderr log to standard error instead of files (default true) - --skip_headers If true, avoid header prefixes in the log messages - --skip_log_headers If true, avoid headers when opening log files - --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 -``` - ### Kubeconfig You can set path to the kubeconfig file by the option or the environment variable just like kubectl. diff --git a/pkg/adaptors/cmd/authentication.go b/pkg/adaptors/cmd/authentication.go index 7f98ea1f..c9c800de 100644 --- a/pkg/adaptors/cmd/authentication.go +++ b/pkg/adaptors/cmd/authentication.go @@ -43,15 +43,18 @@ var allGrantType = strings.Join([]string{ }, "|") func (o *authenticationOptions) addFlags(f *pflag.FlagSet) { - f.StringVar(&o.GrantType, "grant-type", "auto", fmt.Sprintf("The authorization grant type to use. One of (%s)", allGrantType)) - f.StringSliceVar(&o.ListenAddress, "listen-address", defaultListenAddress, "Address to bind to the local server. If multiple addresses are given, it will try binding in order") + f.StringVar(&o.GrantType, "grant-type", "auto", fmt.Sprintf("Authorization grant type to use. One of (%s)", allGrantType)) + f.StringSliceVar(&o.ListenAddress, "listen-address", defaultListenAddress, "[authcode] Address to bind to the local server. If multiple addresses are set, it will try binding in order") //TODO: remove the deprecated flag - f.IntSliceVar(&o.ListenPort, "listen-port", nil, "(Deprecated: use --listen-address)") - f.BoolVar(&o.SkipOpenBrowser, "skip-open-browser", false, "If true, it does not open the browser on authentication") - f.StringVar(&o.RedirectURLHostname, "oidc-redirect-url-hostname", "localhost", "Hostname of the redirect URL") - f.StringToStringVar(&o.AuthRequestExtraParams, "oidc-auth-request-extra-params", nil, "Extra query parameters to send with an authentication request") - 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.IntSliceVar(&o.ListenPort, "listen-port", nil, "[authcode] deprecated: port to bind to the local server") + if err := f.MarkDeprecated("listen-port", "use --listen-address instead"); err != nil { + panic(err) + } + f.BoolVar(&o.SkipOpenBrowser, "skip-open-browser", false, "[authcode] Do not open the browser automatically") + f.StringVar(&o.RedirectURLHostname, "oidc-redirect-url-hostname", "localhost", "[authcode] Hostname of the redirect URL") + f.StringToStringVar(&o.AuthRequestExtraParams, "oidc-auth-request-extra-params", nil, "[authcode, authcode-keyboard] Extra query parameters to send with an authentication request") + f.StringVar(&o.Username, "username", "", "[password] Username for resource owner password credentials grant") + f.StringVar(&o.Password, "password", "", "[password] Password for resource owner password credentials grant") } func (o *authenticationOptions) grantOptionSet() (s authentication.GrantOptionSet, err error) { diff --git a/pkg/adaptors/cmd/root.go b/pkg/adaptors/cmd/root.go index 44f6c4a5..ffec71d6 100644 --- a/pkg/adaptors/cmd/root.go +++ b/pkg/adaptors/cmd/root.go @@ -9,10 +9,10 @@ import ( "golang.org/x/xerrors" ) -const longDescription = `Login to the OpenID Connect provider. +const rootDescription = `Log in 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: +To show the setup instruction: kubectl oidc-login setup @@ -30,8 +30,8 @@ type rootOptions struct { func (o *rootOptions) addFlags(f *pflag.FlagSet) { 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.Context, "context", "", "Name of the kubeconfig context to use") + f.StringVar(&o.User, "user", "", "Name of the kubeconfig user to use. Prior to --context") o.tlsOptions.addFlags(f) o.authenticationOptions.addFlags(f) } @@ -45,8 +45,8 @@ func (cmd *Root) New() *cobra.Command { var o rootOptions c := &cobra.Command{ Use: "kubelogin", - Short: "Login to the OpenID Connect provider", - Long: longDescription, + Short: "Log in to the OpenID Connect provider", + Long: rootDescription, Args: cobra.NoArgs, RunE: func(c *cobra.Command, _ []string) error { grantOptionSet, err := o.authenticationOptions.grantOptionSet() From 923a4251f15094a848c5a29f02fd4b27da2a85c1 Mon Sep 17 00:00:00 2001 From: Hidetake Iwata Date: Sun, 26 Jul 2020 18:11:35 +0900 Subject: [PATCH 5/5] Change messages in standalone mode --- pkg/usecases/standalone/standalone.go | 79 ++++----------------------- 1 file changed, 12 insertions(+), 67 deletions(-) diff --git a/pkg/usecases/standalone/standalone.go b/pkg/usecases/standalone/standalone.go index 4acbb488..319e0056 100644 --- a/pkg/usecases/standalone/standalone.go +++ b/pkg/usecases/standalone/standalone.go @@ -2,8 +2,6 @@ package standalone import ( "context" - "strings" - "text/template" "github.com/google/wire" "github.com/int128/kubelogin/pkg/adaptors/certpool" @@ -36,7 +34,17 @@ type Input struct { GrantOptionSet authentication.GrantOptionSet } -const oidcConfigErrorMessage = `You need to set up the kubeconfig for OpenID Connect authentication. +const oidcConfigErrorMessage = `No configuration found. +You need to set up the OIDC provider, role binding, Kubernetes API server and kubeconfig. +To show the setup instruction: + + kubectl oidc-login setup + +See https://github.com/int128/kubelogin for more. +` + +const deprecationMessage = `NOTE: You can use the credential plugin mode for better user experience. +Kubectl automatically runs kubelogin and you do not need to run kubelogin explicitly. See https://github.com/int128/kubelogin for more. ` @@ -61,9 +69,7 @@ func (u *Standalone) Do(ctx context.Context, in Input) error { u.Logger.Printf(oidcConfigErrorMessage) return xerrors.Errorf("could not find the current authentication provider: %w", err) } - if err := u.showDeprecation(in, authProvider); err != nil { - return xerrors.Errorf("could not show deprecation message: %w", err) - } + u.Logger.Printf(deprecationMessage) u.Logger.V(1).Infof("using the authentication provider of the user %s", authProvider.UserName) u.Logger.V(1).Infof("a token will be written to %s", authProvider.LocationOfOrigin) certPool := u.NewCertPool() @@ -116,64 +122,3 @@ func (u *Standalone) Do(ctx context.Context, in Input) error { } return nil } - -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 the following command: - - kubectl config set-credentials oidc \ - --exec-api-version=client.authentication.k8s.io/v1beta1 \ - --exec-command=kubectl \ - --exec-arg=oidc-login \ - --exec-arg=get-token \ -{{- range .Args }} - --exec-arg={{ . }} -{{- end }} - kubectl config set-context --current --user=oidc - -See https://github.com/int128/kubelogin for more. - -`)) - -type deprecationVars struct { - Args []string -} - -func (u *Standalone) showDeprecation(in Input, p *kubeconfig.AuthProvider) error { - var args []string - args = append(args, "--oidc-issuer-url="+p.IDPIssuerURL) - args = append(args, "--oidc-client-id="+p.ClientID) - if p.ClientSecret != "" { - args = append(args, "--oidc-client-secret="+p.ClientSecret) - } - for _, extraScope := range p.ExtraScopes { - args = append(args, "--oidc-extra-scope="+extraScope) - } - if p.IDPCertificateAuthority != "" { - args = append(args, "--certificate-authority="+p.IDPCertificateAuthority) - } - if p.IDPCertificateAuthorityData != "" { - args = append(args, "--certificate-authority-data="+p.IDPCertificateAuthorityData) - } - if in.CACertFilename != "" { - args = append(args, "--certificate-authority="+in.CACertFilename) - } - if in.GrantOptionSet.ROPCOption != nil { - if in.GrantOptionSet.ROPCOption.Username != "" { - args = append(args, "--username="+in.GrantOptionSet.ROPCOption.Username) - } - } - - v := deprecationVars{ - Args: args, - } - var b strings.Builder - if err := deprecationTpl.Execute(&b, &v); err != nil { - return xerrors.Errorf("template error: %w", err) - } - u.Logger.Printf("%s", b.String()) - return nil -}