diff --git a/pkg/kubeapiserver/options/admission_test.go b/pkg/kubeapiserver/options/admission_test.go index 50672bf5f0c..d71590da75a 100644 --- a/pkg/kubeapiserver/options/admission_test.go +++ b/pkg/kubeapiserver/options/admission_test.go @@ -19,6 +19,9 @@ package options import ( "reflect" "testing" + + "github.com/spf13/pflag" + "github.com/stretchr/testify/assert" ) func TestValidate(t *testing.T) { @@ -51,6 +54,12 @@ func TestValidate(t *testing.T) { if errs := options.Validate(); len(errs) > 0 { t.Errorf("Unexpected err: %v", errs) } + + // nil pointer + options = nil + if errs := options.Validate(); errs != nil { + t.Errorf("expected no errors, error found %+v", errs) + } } func TestComputeEnabledAdmission(t *testing.T) { @@ -86,3 +95,22 @@ func TestComputeEnabledAdmission(t *testing.T) { }) } } + +func TestAdmissionOptionsAddFlags(t *testing.T) { + var args = []string{ + "--enable-admission-plugins=foo,bar,baz", + "--admission-control-config-file=admission_control_config.yaml", + } + + opts := NewAdmissionOptions() + pf := pflag.NewFlagSet("test-admission-opts", pflag.ContinueOnError) + opts.AddFlags(pf) + + if err := pf.Parse(args); err != nil { + t.Fatal(err) + } + + // using assert because cannot compare neither pointer nor function of underlying GenericAdmission + assert.Equal(t, opts.GenericAdmission.ConfigFile, "admission_control_config.yaml") + assert.Equal(t, opts.GenericAdmission.EnablePlugins, []string{"foo", "bar", "baz"}) +} diff --git a/pkg/kubeapiserver/options/authentication_test.go b/pkg/kubeapiserver/options/authentication_test.go index a431e3a8acf..65467e51245 100644 --- a/pkg/kubeapiserver/options/authentication_test.go +++ b/pkg/kubeapiserver/options/authentication_test.go @@ -23,8 +23,10 @@ import ( "time" "github.com/google/go-cmp/cmp" + "github.com/spf13/pflag" utilerrors "k8s.io/apimachinery/pkg/util/errors" + "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apiserver/pkg/authentication/authenticator" "k8s.io/apiserver/pkg/authentication/authenticatorfactory" "k8s.io/apiserver/pkg/authentication/request/headerrequest" @@ -34,10 +36,11 @@ import ( func TestAuthenticationValidate(t *testing.T) { testCases := []struct { - name string - testOIDC *OIDCAuthenticationOptions - testSA *ServiceAccountAuthenticationOptions - expectErr string + name string + testOIDC *OIDCAuthenticationOptions + testSA *ServiceAccountAuthenticationOptions + testWebHook *WebHookAuthenticationOptions + expectErr string }{ { name: "test when OIDC and ServiceAccounts are nil", @@ -133,6 +136,62 @@ func TestAuthenticationValidate(t *testing.T) { }, expectErr: "service-account-issuer \"http://[::1]:namedport\" contained a ':' but was not a valid URL", }, + { + name: "test when ServiceAccounts has invalid JWKSURI", + testOIDC: &OIDCAuthenticationOptions{ + UsernameClaim: "sub", + SigningAlgs: []string{"RS256"}, + IssuerURL: "testIssuerURL", + ClientID: "testClientID", + }, + testSA: &ServiceAccountAuthenticationOptions{ + KeyFiles: []string{"cert", "key"}, + Issuers: []string{"http://foo.bar.com"}, + JWKSURI: "https://host:port", + }, + expectErr: "service-account-jwks-uri must be a valid URL: parse \"https://host:port\": invalid port \":port\" after host", + }, + { + name: "test when ServiceAccounts has invalid JWKSURI (not https scheme)", + testOIDC: &OIDCAuthenticationOptions{ + UsernameClaim: "sub", + SigningAlgs: []string{"RS256"}, + IssuerURL: "testIssuerURL", + ClientID: "testClientID", + }, + testSA: &ServiceAccountAuthenticationOptions{ + KeyFiles: []string{"cert", "key"}, + Issuers: []string{"http://foo.bar.com"}, + JWKSURI: "http://baz.com", + }, + expectErr: "service-account-jwks-uri requires https scheme, parsed as: http://baz.com", + }, + { + name: "test when WebHook has invalid retry attempts", + testOIDC: &OIDCAuthenticationOptions{ + UsernameClaim: "sub", + SigningAlgs: []string{"RS256"}, + IssuerURL: "testIssuerURL", + ClientID: "testClientID", + }, + testSA: &ServiceAccountAuthenticationOptions{ + KeyFiles: []string{"cert", "key"}, + Issuers: []string{"http://foo.bar.com"}, + JWKSURI: "https://baz.com", + }, + testWebHook: &WebHookAuthenticationOptions{ + ConfigFile: "configfile", + Version: "v1", + CacheTTL: 60 * time.Second, + RetryBackoff: &wait.Backoff{ + Duration: 500 * time.Millisecond, + Factor: 1.5, + Jitter: 0.2, + Steps: 0, + }, + }, + expectErr: "number of webhook retry attempts must be greater than 0, but is: 0", + }, } for _, testcase := range testCases { @@ -140,6 +199,7 @@ func TestAuthenticationValidate(t *testing.T) { options := NewBuiltInAuthenticationOptions() options.OIDC = testcase.testOIDC options.ServiceAccounts = testcase.testSA + options.WebHook = testcase.testWebHook errs := options.Validate() if len(errs) > 0 && (!strings.Contains(utilerrors.NewAggregate(errs).Error(), testcase.expectErr) || testcase.expectErr == "") { @@ -239,3 +299,89 @@ func TestToAuthenticationConfig(t *testing.T) { t.Error(cmp.Diff(resultConfig, expectConfig)) } } + +func TestBuiltInAuthenticationOptionsAddFlags(t *testing.T) { + var args = []string{ + "--api-audiences=foo", + "--anonymous-auth=true", + "--enable-bootstrap-token-auth=true", + "--oidc-issuer-url=https://baz.com", + "--oidc-client-id=client-id", + "--oidc-ca-file=cert", + "--oidc-username-prefix=-", + "--client-ca-file=client-cacert", + "--requestheader-client-ca-file=testdata/root.pem", + "--requestheader-username-headers=x-remote-user-custom", + "--requestheader-group-headers=x-remote-group-custom", + "--requestheader-allowed-names=kube-aggregator", + "--service-account-key-file=cert", + "--service-account-key-file=key", + "--service-account-issuer=http://foo.bar.com", + "--service-account-jwks-uri=https://qux.com", + "--token-auth-file=tokenfile", + "--authentication-token-webhook-config-file=webhook_config.yaml", + "--authentication-token-webhook-cache-ttl=180s", + } + + expected := &BuiltInAuthenticationOptions{ + APIAudiences: []string{"foo"}, + Anonymous: &AnonymousAuthenticationOptions{ + Allow: true, + }, + BootstrapToken: &BootstrapTokenAuthenticationOptions{ + Enable: true, + }, + ClientCert: &apiserveroptions.ClientCertAuthenticationOptions{ + ClientCA: "client-cacert", + }, + OIDC: &OIDCAuthenticationOptions{ + CAFile: "cert", + ClientID: "client-id", + IssuerURL: "https://baz.com", + UsernameClaim: "sub", + UsernamePrefix: "-", + SigningAlgs: []string{"RS256"}, + }, + RequestHeader: &apiserveroptions.RequestHeaderAuthenticationOptions{ + ClientCAFile: "testdata/root.pem", + UsernameHeaders: []string{"x-remote-user-custom"}, + GroupHeaders: []string{"x-remote-group-custom"}, + AllowedNames: []string{"kube-aggregator"}, + }, + ServiceAccounts: &ServiceAccountAuthenticationOptions{ + KeyFiles: []string{"cert", "key"}, + Lookup: true, + Issuers: []string{"http://foo.bar.com"}, + JWKSURI: "https://qux.com", + ExtendExpiration: true, + }, + TokenFile: &TokenFileAuthenticationOptions{ + TokenFile: "tokenfile", + }, + WebHook: &WebHookAuthenticationOptions{ + ConfigFile: "webhook_config.yaml", + Version: "v1beta1", + CacheTTL: 180 * time.Second, + RetryBackoff: &wait.Backoff{ + Duration: 500 * time.Millisecond, + Factor: 1.5, + Jitter: 0.2, + Steps: 5, + }, + }, + TokenSuccessCacheTTL: 10 * time.Second, + TokenFailureCacheTTL: 0 * time.Second, + } + + opts := NewBuiltInAuthenticationOptions().WithAll() + pf := pflag.NewFlagSet("test-builtin-authentication-opts", pflag.ContinueOnError) + opts.AddFlags(pf) + + if err := pf.Parse(args); err != nil { + t.Fatal(err) + } + + if !reflect.DeepEqual(opts, expected) { + t.Error(cmp.Diff(opts, expected)) + } +} diff --git a/pkg/kubeapiserver/options/authorization_test.go b/pkg/kubeapiserver/options/authorization_test.go index 4794de700cb..97ef24ac2d1 100644 --- a/pkg/kubeapiserver/options/authorization_test.go +++ b/pkg/kubeapiserver/options/authorization_test.go @@ -17,8 +17,17 @@ limitations under the License. package options import ( + "fmt" + "reflect" + "strings" "testing" + "time" + "github.com/google/go-cmp/cmp" + "github.com/spf13/pflag" + + utilerrors "k8s.io/apimachinery/pkg/util/errors" + "k8s.io/apimachinery/pkg/util/wait" "k8s.io/kubernetes/pkg/kubeapiserver/authorizer/modes" ) @@ -26,21 +35,31 @@ func TestAuthzValidate(t *testing.T) { examplePolicyFile := "../../auth/authorizer/abac/example_policy_file.jsonl" testCases := []struct { - name string - modes []string - policyFile string - webhookConfigFile string - expectErr bool + name string + modes []string + policyFile string + webhookConfigFile string + webhookRetryBackoff *wait.Backoff + expectErr bool + expectErrorSubString string }{ { - name: "Unknown modes should return errors", - modes: []string{"DoesNotExist"}, - expectErr: true, + name: "Unknown modes should return errors", + modes: []string{"DoesNotExist"}, + expectErr: true, + expectErrorSubString: "is not a valid mode", }, { - name: "At least one authorizationMode is necessary", - modes: []string{}, - expectErr: true, + name: "At least one authorizationMode is necessary", + modes: []string{}, + expectErr: true, + expectErrorSubString: "at least one authorization-mode must be passed", + }, + { + name: "ModeAlwaysAllow specified more than once", + modes: []string{modes.ModeAlwaysAllow, modes.ModeAlwaysAllow}, + expectErr: true, + expectErrorSubString: "has mode specified more than once", }, { name: "ModeAlwaysAllow and ModeAlwaysDeny should return without authorizationPolicyFile", @@ -48,16 +67,18 @@ func TestAuthzValidate(t *testing.T) { expectErr: false, }, { - name: "ModeABAC requires a policy file", - modes: []string{modes.ModeAlwaysAllow, modes.ModeAlwaysDeny, modes.ModeABAC}, - expectErr: true, + name: "ModeABAC requires a policy file", + modes: []string{modes.ModeAlwaysAllow, modes.ModeAlwaysDeny, modes.ModeABAC}, + expectErr: true, + expectErrorSubString: "authorization-mode ABAC's authorization policy file not passed", }, { - name: "Authorization Policy file cannot be used without ModeABAC", - modes: []string{modes.ModeAlwaysAllow, modes.ModeAlwaysDeny}, - policyFile: examplePolicyFile, - webhookConfigFile: "", - expectErr: true, + name: "Authorization Policy file cannot be used without ModeABAC", + modes: []string{modes.ModeAlwaysAllow, modes.ModeAlwaysDeny}, + policyFile: examplePolicyFile, + webhookConfigFile: "", + expectErr: true, + expectErrorSubString: "cannot specify --authorization-policy-file without mode ABAC", }, { name: "ModeABAC should not error if a valid policy path is provided", @@ -67,15 +88,17 @@ func TestAuthzValidate(t *testing.T) { expectErr: false, }, { - name: "ModeWebhook requires a config file", - modes: []string{modes.ModeWebhook}, - expectErr: true, + name: "ModeWebhook requires a config file", + modes: []string{modes.ModeWebhook}, + expectErr: true, + expectErrorSubString: "authorization-mode Webhook's authorization config file not passed", }, { - name: "Cannot provide webhook config file without ModeWebhook", - modes: []string{modes.ModeAlwaysAllow}, - webhookConfigFile: "authz_webhook_config.yaml", - expectErr: true, + name: "Cannot provide webhook config file without ModeWebhook", + modes: []string{modes.ModeAlwaysAllow}, + webhookConfigFile: "authz_webhook_config.yaml", + expectErr: true, + expectErrorSubString: "cannot specify --authorization-webhook-config-file without mode Webhook", }, { name: "ModeWebhook should not error if a valid config file is provided", @@ -83,6 +106,14 @@ func TestAuthzValidate(t *testing.T) { webhookConfigFile: "authz_webhook_config.yaml", expectErr: false, }, + { + name: "ModeWebhook should error if an invalid number of webhook retry attempts is provided", + modes: []string{modes.ModeWebhook}, + webhookConfigFile: "authz_webhook_config.yaml", + webhookRetryBackoff: &wait.Backoff{Steps: 0}, + expectErr: true, + expectErrorSubString: "number of webhook retry attempts must be greater than 0", + }, } for _, testcase := range testCases { @@ -90,6 +121,7 @@ func TestAuthzValidate(t *testing.T) { options := NewBuiltInAuthorizationOptions() options.Modes = testcase.modes options.WebhookConfigFile = testcase.webhookConfigFile + options.WebhookRetryBackoff = testcase.webhookRetryBackoff options.PolicyFile = testcase.policyFile errs := options.Validate() @@ -99,6 +131,49 @@ func TestAuthzValidate(t *testing.T) { if testcase.expectErr && len(errs) == 0 { t.Errorf("should return an error") } + if len(errs) > 0 && testcase.expectErr { + if !strings.Contains(utilerrors.NewAggregate(errs).Error(), testcase.expectErrorSubString) { + t.Errorf("exepected to found error: %s, but no error found", testcase.expectErrorSubString) + } + } }) } } + +func TestBuiltInAuthorizationOptionsAddFlags(t *testing.T) { + var args = []string{ + fmt.Sprintf("--authorization-mode=%s,%s,%s,%s", modes.ModeAlwaysAllow, modes.ModeAlwaysDeny, modes.ModeABAC, modes.ModeWebhook), + "--authorization-policy-file=policy_file.json", + "--authorization-webhook-config-file=webhook_config_file.yaml", + "--authorization-webhook-version=v1", + "--authorization-webhook-cache-authorized-ttl=60s", + "--authorization-webhook-cache-unauthorized-ttl=30s", + } + + expected := &BuiltInAuthorizationOptions{ + Modes: []string{modes.ModeAlwaysAllow, modes.ModeAlwaysDeny, modes.ModeABAC, modes.ModeWebhook}, + PolicyFile: "policy_file.json", + WebhookConfigFile: "webhook_config_file.yaml", + WebhookVersion: "v1", + WebhookCacheAuthorizedTTL: 60 * time.Second, + WebhookCacheUnauthorizedTTL: 30 * time.Second, + WebhookRetryBackoff: &wait.Backoff{ + Duration: 500 * time.Millisecond, + Factor: 1.5, + Jitter: 0.2, + Steps: 5, + }, + } + + opts := NewBuiltInAuthorizationOptions() + pf := pflag.NewFlagSet("test-builtin-authorization-opts", pflag.ContinueOnError) + opts.AddFlags(pf) + + if err := pf.Parse(args); err != nil { + t.Fatal(err) + } + + if !reflect.DeepEqual(opts, expected) { + t.Error(cmp.Diff(opts, expected)) + } +}