From 45f57939af00be6e6d32331b1c854b13a04ee4b0 Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Tue, 2 Mar 2021 14:17:27 -0600 Subject: [PATCH 1/4] Make TestGetPinnipedCategory more resilient. If the test is run immediately after the Concierge is installed, the API server can still have broken discovery data and return an error on the first call. This commit adds a retry loop to attempt this first kubectl command for up to 60s before declaring failure. The subsequent tests should be covered by this as well since they are not run in parallel. Signed-off-by: Matt Moyer --- test/integration/category_test.go | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/test/integration/category_test.go b/test/integration/category_test.go index e8edc7bd..42d44582 100644 --- a/test/integration/category_test.go +++ b/test/integration/category_test.go @@ -7,6 +7,7 @@ import ( "bytes" "os/exec" "testing" + "time" "github.com/stretchr/testify/require" @@ -20,13 +21,21 @@ func TestGetPinnipedCategory(t *testing.T) { t.Run("category, no special params", func(t *testing.T) { var stdOut, stdErr bytes.Buffer - cmd := exec.Command("kubectl", "get", "pinniped", "-A") - cmd.Stdout = &stdOut - cmd.Stderr = &stdErr - err := cmd.Run() - require.NoError(t, err, stdErr.String(), stdOut.String()) + var err error + require.Eventuallyf(t, func() bool { + cmd := exec.Command("kubectl", "get", "pinniped", "-A") + cmd.Stdout = &stdOut + cmd.Stderr = &stdErr + err = cmd.Run() + return err == nil + }, + 60*time.Second, + 1*time.Second, + "never ran 'kubectl get pinniped -A' successfully:\n%s\n\n%s", + stdErr.String(), + stdOut.String(), + ) require.Empty(t, stdErr.String()) - require.NotContains(t, stdOut.String(), "MethodNotAllowed") require.Contains(t, stdOut.String(), dotSuffix) }) From df27c2e1fca30972457d7bd4c97d9261ab023f6b Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Tue, 2 Mar 2021 15:41:21 -0600 Subject: [PATCH 2/4] Use randomly generated API groups in TestKubeClientOwnerRef. I think this is another aspect of the test flakes we're trying to fix. This matters especially for the "Multiple Pinnipeds" test environment where two copies of the test suite are running concurrently. Signed-off-by: Matt Moyer --- test/integration/kubeclient_test.go | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/test/integration/kubeclient_test.go b/test/integration/kubeclient_test.go index 6cdaa56d..e5f759c5 100644 --- a/test/integration/kubeclient_test.go +++ b/test/integration/kubeclient_test.go @@ -5,6 +5,7 @@ package integration import ( "context" + "fmt" "testing" "time" @@ -75,15 +76,18 @@ func TestKubeClientOwnerRef(t *testing.T) { UID: parentSecret.UID, } + snorlaxAPIGroup := fmt.Sprintf("%s.snorlax.dev", library.RandHex(t, 8)) parentAPIService, err := regularAggregationClient.ApiregistrationV1().APIServices().Create( ctx, &apiregistrationv1.APIService{ ObjectMeta: metav1.ObjectMeta{ - Name: "v1.snorlax.dev", + Name: "v1." + snorlaxAPIGroup, + Labels: map[string]string{"pinniped.dev/test": ""}, + Annotations: map[string]string{"pinniped.dev/testName": t.Name()}, }, Spec: apiregistrationv1.APIServiceSpec{ Version: "v1", - Group: "snorlax.dev", + Group: snorlaxAPIGroup, GroupPriorityMinimum: 10_000, VersionPriority: 500, }, @@ -183,16 +187,19 @@ func TestKubeClientOwnerRef(t *testing.T) { }) // cluster scoped API service should be owned by the other one we created above + pandasAPIGroup := fmt.Sprintf("%s.pandas.dev", library.RandHex(t, 8)) apiService, err := ownerRefClient.Aggregation.ApiregistrationV1().APIServices().Create( ctx, &apiregistrationv1.APIService{ ObjectMeta: metav1.ObjectMeta{ - Name: "v1.pandas.dev", + Name: "v1." + pandasAPIGroup, OwnerReferences: nil, // no owner refs set + Labels: map[string]string{"pinniped.dev/test": ""}, + Annotations: map[string]string{"pinniped.dev/testName": t.Name()}, }, Spec: apiregistrationv1.APIServiceSpec{ Version: "v1", - Group: "pandas.dev", + Group: pandasAPIGroup, GroupPriorityMinimum: 10_000, VersionPriority: 500, }, From d7edc41c24786e223d1f01dca15270129115c93a Mon Sep 17 00:00:00 2001 From: Monis Khan Date: Wed, 3 Mar 2021 13:37:43 -0500 Subject: [PATCH 3/4] oidc discovery: encode metadata once and reuse Signed-off-by: Monis Khan --- internal/oidc/discovery/discovery_handler.go | 39 +++++++++++++------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/internal/oidc/discovery/discovery_handler.go b/internal/oidc/discovery/discovery_handler.go index 5eb6c481..b45c1042 100644 --- a/internal/oidc/discovery/discovery_handler.go +++ b/internal/oidc/discovery/discovery_handler.go @@ -5,6 +5,7 @@ package discovery import ( + "bytes" "encoding/json" "net/http" @@ -40,28 +41,38 @@ type Metadata struct { // NewHandler returns an http.Handler that serves an OIDC discovery endpoint. func NewHandler(issuerURL string) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") + oidcConfig := Metadata{ + Issuer: issuerURL, + AuthorizationEndpoint: issuerURL + oidc.AuthorizationEndpointPath, + TokenEndpoint: issuerURL + oidc.TokenEndpointPath, + JWKSURI: issuerURL + oidc.JWKSEndpointPath, + ResponseTypesSupported: []string{"code"}, + SubjectTypesSupported: []string{"public"}, + IDTokenSigningAlgValuesSupported: []string{"ES256"}, + TokenEndpointAuthMethodsSupported: []string{"client_secret_basic"}, + ScopesSupported: []string{"openid", "offline"}, + ClaimsSupported: []string{"groups"}, + } + var b bytes.Buffer + encodeErr := json.NewEncoder(&b).Encode(&oidcConfig) + encodedMetadata := b.Bytes() + + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.Method != http.MethodGet { http.Error(w, `Method not allowed (try GET)`, http.StatusMethodNotAllowed) return } - oidcConfig := Metadata{ - Issuer: issuerURL, - AuthorizationEndpoint: issuerURL + oidc.AuthorizationEndpointPath, - TokenEndpoint: issuerURL + oidc.TokenEndpointPath, - JWKSURI: issuerURL + oidc.JWKSEndpointPath, - ResponseTypesSupported: []string{"code"}, - SubjectTypesSupported: []string{"public"}, - IDTokenSigningAlgValuesSupported: []string{"ES256"}, - TokenEndpointAuthMethodsSupported: []string{"client_secret_basic"}, - ScopesSupported: []string{"openid", "offline"}, - ClaimsSupported: []string{"groups"}, + if encodeErr != nil { + http.Error(w, encodeErr.Error(), http.StatusInternalServerError) + return } - if err := json.NewEncoder(w).Encode(&oidcConfig); err != nil { + + w.Header().Set("Content-Type", "application/json") + if _, err := w.Write(encodedMetadata); err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) + return } }) } From 14b8def3208a157d37c43a781c3bd29bf2bd596b Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 4 Mar 2021 06:10:36 +0000 Subject: [PATCH 4/4] Bump k8s.io/klog/v2 from 2.5.0 to 2.6.0 Bumps [k8s.io/klog/v2](https://github.com/kubernetes/klog) from 2.5.0 to 2.6.0. - [Release notes](https://github.com/kubernetes/klog/releases) - [Changelog](https://github.com/kubernetes/klog/blob/master/RELEASE.md) - [Commits](https://github.com/kubernetes/klog/compare/v2.5.0...v2.6.0) Signed-off-by: dependabot[bot] --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index eb6c811c..3198912d 100644 --- a/go.mod +++ b/go.mod @@ -36,7 +36,7 @@ require ( k8s.io/client-go v0.20.4 k8s.io/component-base v0.20.4 k8s.io/gengo v0.0.0-20201113003025-83324d819ded - k8s.io/klog/v2 v2.5.0 + k8s.io/klog/v2 v2.6.0 k8s.io/kube-aggregator v0.20.4 k8s.io/kube-openapi v0.0.0-20201113171705-d219536bb9fd k8s.io/utils v0.0.0-20201110183641-67b214c5f920 diff --git a/go.sum b/go.sum index 1d0de09f..8abae75f 100644 --- a/go.sum +++ b/go.sum @@ -1528,8 +1528,8 @@ k8s.io/gengo v0.0.0-20201113003025-83324d819ded/go.mod h1:FiNAH4ZV3gBg2Kwh89tzAE k8s.io/klog/v2 v2.0.0/go.mod h1:PBfzABfn139FHAV07az/IF9Wp1bkk3vpT2XSJ76fSDE= k8s.io/klog/v2 v2.2.0/go.mod h1:Od+F08eJP+W3HUb4pSrPpgp9DGU4GzlpG/TmITuYh/Y= k8s.io/klog/v2 v2.4.0/go.mod h1:Od+F08eJP+W3HUb4pSrPpgp9DGU4GzlpG/TmITuYh/Y= -k8s.io/klog/v2 v2.5.0 h1:8mOnjf1RmUPW6KRqQCfYSZq/K20Unmp3IhuZUhxl8KI= -k8s.io/klog/v2 v2.5.0/go.mod h1:hy9LJ/NvuK+iVyP4Ehqva4HxZG/oXyIS3n3Jmire4Ec= +k8s.io/klog/v2 v2.6.0 h1:c1wFxejFMBkp/VxCdc6kYdgrBkC2gzmcl6afuJAkJyU= +k8s.io/klog/v2 v2.6.0/go.mod h1:hy9LJ/NvuK+iVyP4Ehqva4HxZG/oXyIS3n3Jmire4Ec= k8s.io/kube-aggregator v0.20.4 h1:j/SUwPy1eO+ud3XOUGmH18gISPyerqhXOoNRZDbv3fs= k8s.io/kube-aggregator v0.20.4/go.mod h1:0ixQ9De7KXyHteXizS6nVtrnKqGa4kiuxl9rEBsNccw= k8s.io/kube-openapi v0.0.0-20201113171705-d219536bb9fd h1:sOHNzJIkytDF6qadMNKhhDRpc6ODik8lVC6nOur7B2c=