From c87f091a442181c9018463e4da2e89da86a46d0f Mon Sep 17 00:00:00 2001 From: Joshua Casey Date: Tue, 3 Sep 2024 06:45:08 -0500 Subject: [PATCH 1/5] Upcoming k8s versions have an additional extra field in the CSR response - failure due to https://github.com/kubernetes/kubernetes/pull/125634 --- .../testutil/kube_server_compatibility.go | 9 ++ test/integration/concierge_whoami_test.go | 92 ++++++++++--------- 2 files changed, 58 insertions(+), 43 deletions(-) diff --git a/internal/testutil/kube_server_compatibility.go b/internal/testutil/kube_server_compatibility.go index 91764e57f..e39bcd9d7 100644 --- a/internal/testutil/kube_server_compatibility.go +++ b/internal/testutil/kube_server_compatibility.go @@ -32,6 +32,15 @@ func KubeServerSupportsCertificatesV1API(t *testing.T, discoveryClient discovery return false } +func PrintKubeServerVersion(t *testing.T, discoveryClient discovery.DiscoveryInterface) { + t.Helper() + + version, err := discoveryClient.ServerVersion() + require.NoError(t, err) + + t.Logf("K8s server version: %s\n%+v", version, version) +} + func KubeServerMinorVersionAtLeastInclusive(t *testing.T, discoveryClient discovery.DiscoveryInterface, min int) bool { return !KubeServerMinorVersionInBetweenInclusive(t, discoveryClient, 0, min-1) } diff --git a/test/integration/concierge_whoami_test.go b/test/integration/concierge_whoami_test.go index 3993a7984..a5c922452 100644 --- a/test/integration/concierge_whoami_test.go +++ b/test/integration/concierge_whoami_test.go @@ -10,16 +10,17 @@ import ( "crypto/x509" "crypto/x509/pkix" "encoding/pem" + "fmt" "testing" "time" "github.com/stretchr/testify/require" authenticationv1 "k8s.io/api/authentication/v1" certificatesv1 "k8s.io/api/certificates/v1" - certificatesv1beta1 "k8s.io/api/certificates/v1beta1" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/discovery" "k8s.io/client-go/rest" "k8s.io/client-go/util/cert" "k8s.io/client-go/util/certificate/csr" @@ -62,6 +63,8 @@ func TestWhoAmI_Kubeadm_Parallel(t *testing.T) { User: identityv1alpha1.UserInfo{ Username: "kubernetes-admin", Groups: wantGroups, + // Starting in 1.32, Kubernetes returns us a dynamically-computed SHA256 that we can't predict here. + Extra: whoAmI.Status.KubernetesUserInfo.User.Extra, }, }, }, @@ -298,53 +301,26 @@ func TestWhoAmI_CSR_Parallel(t *testing.T) { ) require.NoError(t, err) - useCertificatesV1API := testutil.KubeServerSupportsCertificatesV1API(t, kubeClient.Discovery()) - t.Cleanup(func() { - if useCertificatesV1API { - require.NoError(t, kubeClient.CertificatesV1().CertificateSigningRequests(). - Delete(context.Background(), csrName, metav1.DeleteOptions{})) - } else { - // On old clusters use v1beta1 - require.NoError(t, kubeClient.CertificatesV1beta1().CertificateSigningRequests(). - Delete(context.Background(), csrName, metav1.DeleteOptions{})) - } + require.NoError(t, kubeClient.CertificatesV1().CertificateSigningRequests(). + Delete(context.Background(), csrName, metav1.DeleteOptions{})) }) - if useCertificatesV1API { - _, err = kubeClient.CertificatesV1().CertificateSigningRequests().UpdateApproval(ctx, csrName, &certificatesv1.CertificateSigningRequest{ - ObjectMeta: metav1.ObjectMeta{ - Name: csrName, - }, - Status: certificatesv1.CertificateSigningRequestStatus{ - Conditions: []certificatesv1.CertificateSigningRequestCondition{ - { - Type: certificatesv1.CertificateApproved, - Status: corev1.ConditionTrue, - Reason: "WhoAmICSRTest", - }, + _, err = kubeClient.CertificatesV1().CertificateSigningRequests().UpdateApproval(ctx, csrName, &certificatesv1.CertificateSigningRequest{ + ObjectMeta: metav1.ObjectMeta{ + Name: csrName, + }, + Status: certificatesv1.CertificateSigningRequestStatus{ + Conditions: []certificatesv1.CertificateSigningRequestCondition{ + { + Type: certificatesv1.CertificateApproved, + Status: corev1.ConditionTrue, + Reason: "WhoAmICSRTest", }, }, - }, metav1.UpdateOptions{}) - require.NoError(t, err) - } else { - // On old Kubernetes clusters use CertificatesV1beta1 - _, err = kubeClient.CertificatesV1beta1().CertificateSigningRequests().UpdateApproval(ctx, &certificatesv1beta1.CertificateSigningRequest{ - ObjectMeta: metav1.ObjectMeta{ - Name: csrName, - }, - Status: certificatesv1beta1.CertificateSigningRequestStatus{ - Conditions: []certificatesv1beta1.CertificateSigningRequestCondition{ - { - Type: certificatesv1beta1.CertificateApproved, - Status: corev1.ConditionTrue, - Reason: "WhoAmICSRTest", - }, - }, - }, - }, metav1.UpdateOptions{}) - require.NoError(t, err) - } + }, + }, metav1.UpdateOptions{}) + require.NoError(t, err) crtPEM, err := csr.WaitForCertificate(ctx, kubeClient, csrName, csrUID) require.NoError(t, err) @@ -357,6 +333,8 @@ func TestWhoAmI_CSR_Parallel(t *testing.T) { Create(ctx, &identityv1alpha1.WhoAmIRequest{}, metav1.CreateOptions{}) require.NoError(t, err, testlib.Sdump(err)) + testutil.PrintKubeServerVersion(t, kubeClient.Discovery()) + require.Equal(t, &identityv1alpha1.WhoAmIRequest{ Status: identityv1alpha1.WhoAmIRequestStatus{ @@ -368,6 +346,7 @@ func TestWhoAmI_CSR_Parallel(t *testing.T) { "living-the-dream", "system:authenticated", }, + Extra: maybeNeedsExtraWithSHA256(t, kubeClient.Discovery(), crtPEM), }, }, }, @@ -376,6 +355,33 @@ func TestWhoAmI_CSR_Parallel(t *testing.T) { ) } +func maybeNeedsExtraWithSHA256( + t *testing.T, + discoveryClient discovery.DiscoveryInterface, + certPEM []byte, +) map[string]identityv1alpha1.ExtraValue { + t.Helper() + var extra map[string]identityv1alpha1.ExtraValue + + if testutil.KubeServerMinorVersionAtLeastInclusive(t, discoveryClient, 32) { + t.Log("Adding extra that includes SHA256 of the DER of the first cert") + block, _ := pem.Decode(certPEM) + require.NotEmpty(t, block) + + bytesToHexOfSHA256 := func(b []byte) string { + shasum := sha256.Sum256(b) + return hex.EncodeToString(shasum[:]) + } + + sha256OfDER := fmt.Sprintf("X509SHA256=%s", bytesToHexOfSHA256(block.Bytes)) + + extra = map[string]identityv1alpha1.ExtraValue{ + "authentication.kubernetes.io/credential-id": []string{sha256OfDER}, + } + } + return extra +} + // whoami requests are non-mutating and safe to run in parallel with serial tests, see main_test.go. func TestWhoAmI_Anonymous_Parallel(t *testing.T) { _ = testlib.IntegrationEnv(t).WithCapability(testlib.AnonymousAuthenticationSupported) From f476259bbfc387140157d0141f74ebd0e6446013 Mon Sep 17 00:00:00 2001 From: Joshua Casey Date: Tue, 3 Sep 2024 06:46:29 -0500 Subject: [PATCH 2/5] Bump all dependencies --- go.mod | 14 ++++++------ go.sum | 28 +++++++++++------------ hack/update-go-mod/go.mod | 2 +- hack/update-go-mod/go.sum | 4 ++-- test/integration/concierge_whoami_test.go | 9 ++++++++ 5 files changed, 33 insertions(+), 24 deletions(-) diff --git a/go.mod b/go.mod index 3539cd383..abf19bc7e 100644 --- a/go.mod +++ b/go.mod @@ -69,21 +69,21 @@ require ( go.uber.org/zap v1.27.0 golang.org/x/crypto v0.26.0 golang.org/x/net v0.28.0 - golang.org/x/oauth2 v0.22.0 + golang.org/x/oauth2 v0.23.0 golang.org/x/sync v0.8.0 - golang.org/x/term v0.23.0 - golang.org/x/text v0.17.0 + golang.org/x/term v0.24.0 + golang.org/x/text v0.18.0 k8s.io/api v0.31.0 k8s.io/apiextensions-apiserver v0.31.0 k8s.io/apimachinery v0.31.0 k8s.io/apiserver v0.31.0 k8s.io/client-go v0.31.0 k8s.io/component-base v0.31.0 - k8s.io/gengo v0.0.0-20240826214909-a7b603a56eb7 + k8s.io/gengo v0.0.0-20240904190049-f173c7c23b06 k8s.io/klog/v2 v2.130.1 k8s.io/kube-aggregator v0.31.0 - k8s.io/kube-openapi v0.0.0-20240827152857-f7e401e7b4c2 - k8s.io/utils v0.0.0-20240821151609-f90d01438635 + k8s.io/kube-openapi v0.0.0-20240903163716-9e1beecbcb38 + k8s.io/utils v0.0.0-20240902221715-702e33fdd3c3 sigs.k8s.io/yaml v1.4.0 ) @@ -186,7 +186,7 @@ require ( go.uber.org/multierr v1.11.0 // indirect golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 // indirect golang.org/x/mod v0.20.0 // indirect - golang.org/x/sys v0.23.0 // indirect + golang.org/x/sys v0.25.0 // indirect golang.org/x/time v0.4.0 // indirect golang.org/x/tools v0.24.0 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20240701130421-f6361c86f094 // indirect diff --git a/go.sum b/go.sum index 3f1dd48be..c7035ddfa 100644 --- a/go.sum +++ b/go.sum @@ -776,8 +776,8 @@ golang.org/x/oauth2 v0.0.0-20200902213428-5d25da1a8d43/go.mod h1:KelEdhl1UZF7XfJ golang.org/x/oauth2 v0.0.0-20201109201403-9fd604954f58/go.mod h1:KelEdhl1UZF7XfJ4dDtk6s++YSgaE7mD/BuKKDLBl4A= golang.org/x/oauth2 v0.0.0-20201208152858-08078c50e5b5/go.mod h1:KelEdhl1UZF7XfJ4dDtk6s++YSgaE7mD/BuKKDLBl4A= golang.org/x/oauth2 v0.0.0-20210218202405-ba52d332ba99/go.mod h1:KelEdhl1UZF7XfJ4dDtk6s++YSgaE7mD/BuKKDLBl4A= -golang.org/x/oauth2 v0.22.0 h1:BzDx2FehcG7jJwgWLELCdmLuxk2i+x9UDpSiss2u0ZA= -golang.org/x/oauth2 v0.22.0/go.mod h1:XYTD2NtWslqkgxebSiOHnXEap4TF09sJSc7H1sXbhtI= +golang.org/x/oauth2 v0.23.0 h1:PbgcYx2W7i4LvjJWEbf0ngHV6qJYr86PkAV3bXdLEbs= +golang.org/x/oauth2 v0.23.0/go.mod h1:XYTD2NtWslqkgxebSiOHnXEap4TF09sJSc7H1sXbhtI= golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= @@ -852,8 +852,8 @@ golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.18.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.22.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= -golang.org/x/sys v0.23.0 h1:YfKFowiIMvtgl1UERQoTPPToxltDeZfbj4H7dVUCwmM= -golang.org/x/sys v0.23.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/sys v0.25.0 h1:r+8e+loiHxRqhXVl6ML1nO3l1+oFoWbnlu2Ehimmi34= +golang.org/x/sys v0.25.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= @@ -863,8 +863,8 @@ golang.org/x/term v0.7.0/go.mod h1:P32HKFT3hSsZrRxla30E9HqToFYAQPCMs/zFMBUFqPY= golang.org/x/term v0.8.0/go.mod h1:xPskH00ivmX89bAKVGSKKtLOWNx2+17Eiy94tnKShWo= golang.org/x/term v0.17.0/go.mod h1:lLRBjIVuehSbZlaOtGMbcMncT+aqLLLmKrsjNrUguwk= golang.org/x/term v0.18.0/go.mod h1:ILwASektA3OnRv7amZ1xhE/KTR+u50pbXfZ03+6Nx58= -golang.org/x/term v0.23.0 h1:F6D4vR+EHoL9/sWAWgAR1H2DcHr4PareCbAaCo1RpuU= -golang.org/x/term v0.23.0/go.mod h1:DgV24QBUrK6jhZXl+20l6UWznPlwAHm1Q1mGHtydmSk= +golang.org/x/term v0.24.0 h1:Mh5cbb+Zk2hqqXNO7S1iTjEphVL+jb8ZWaqh/g+JWkM= +golang.org/x/term v0.24.0/go.mod h1:lOBK/LVxemqiMij05LGJ0tzNr8xlmwBRJ81PX6wVLH8= golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= @@ -876,8 +876,8 @@ golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= golang.org/x/text v0.9.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8= golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= -golang.org/x/text v0.17.0 h1:XtiM5bkSOt+ewxlOE/aE/AKEHibwj/6gvWMl9Rsh0Qc= -golang.org/x/text v0.17.0/go.mod h1:BuEKDfySbSR4drPmRPG/7iBdf8hvFMuRexcpahXilzY= +golang.org/x/text v0.18.0 h1:XvMDiNzPAl0jr17s6W9lcaIhGUfUORdGCNsuLmPG224= +golang.org/x/text v0.18.0/go.mod h1:BuEKDfySbSR4drPmRPG/7iBdf8hvFMuRexcpahXilzY= golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20191024005414-555d28b269f0/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= @@ -1093,8 +1093,8 @@ k8s.io/client-go v0.31.0 h1:QqEJzNjbN2Yv1H79SsS+SWnXkBgVu4Pj3CJQgbx0gI8= k8s.io/client-go v0.31.0/go.mod h1:Y9wvC76g4fLjmU0BA+rV+h2cncoadjvjjkkIGoTLcGU= k8s.io/component-base v0.31.0 h1:/KIzGM5EvPNQcYgwq5NwoQBaOlVFrghoVGr8lG6vNRs= k8s.io/component-base v0.31.0/go.mod h1:TYVuzI1QmN4L5ItVdMSXKvH7/DtvIuas5/mm8YT3rTo= -k8s.io/gengo v0.0.0-20240826214909-a7b603a56eb7 h1:HCbtr1pVu/ElMcTTs18KdMtH5y6f7PQvrjh1QZj3qCI= -k8s.io/gengo v0.0.0-20240826214909-a7b603a56eb7/go.mod h1:FiNAH4ZV3gBg2Kwh89tzAEV2be7d5xI0vBa/VySYy3E= +k8s.io/gengo v0.0.0-20240904190049-f173c7c23b06 h1:nOLHQ014rUc6MverFvifFBCKw28JbWcNFfzjG0KZUCE= +k8s.io/gengo v0.0.0-20240904190049-f173c7c23b06/go.mod h1:FiNAH4ZV3gBg2Kwh89tzAEV2be7d5xI0vBa/VySYy3E= k8s.io/klog/v2 v2.2.0/go.mod h1:Od+F08eJP+W3HUb4pSrPpgp9DGU4GzlpG/TmITuYh/Y= k8s.io/klog/v2 v2.130.1 h1:n9Xl7H1Xvksem4KFG4PYbdQCQxqc/tTUyrgXaOhHSzk= k8s.io/klog/v2 v2.130.1/go.mod h1:3Jpz1GvMt720eyJH1ckRHK1EDfpxISzJ7I9OYgaDtPE= @@ -1102,10 +1102,10 @@ k8s.io/kms v0.31.0 h1:KchILPfB1ZE+ka7223mpU5zeFNkmb45jl7RHnlImUaI= k8s.io/kms v0.31.0/go.mod h1:OZKwl1fan3n3N5FFxnW5C4V3ygrah/3YXeJWS3O6+94= k8s.io/kube-aggregator v0.31.0 h1:3DqSpmqHF8rey7fY+qYXLJms0tYPhxrgWvjpnKVnS0Y= k8s.io/kube-aggregator v0.31.0/go.mod h1:Fa+OVSpMQC7zbTTz7/QG7FXe9jZ8usuJQej5sMdCrkM= -k8s.io/kube-openapi v0.0.0-20240827152857-f7e401e7b4c2 h1:GKE9U8BH16uynoxQii0auTjmmmuZ3O0LFMN6S0lPPhI= -k8s.io/kube-openapi v0.0.0-20240827152857-f7e401e7b4c2/go.mod h1:coRQXBK9NxO98XUv3ZD6AK3xzHCxV6+b7lrquKwaKzA= -k8s.io/utils v0.0.0-20240821151609-f90d01438635 h1:2wThSvJoW/Ncn9TmQEYXRnevZXi2duqHWf5OX9S3zjI= -k8s.io/utils v0.0.0-20240821151609-f90d01438635/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0= +k8s.io/kube-openapi v0.0.0-20240903163716-9e1beecbcb38 h1:1dWzkmJrrprYvjGwh9kEUxmcUV/CtNU8QM7h1FLWQOo= +k8s.io/kube-openapi v0.0.0-20240903163716-9e1beecbcb38/go.mod h1:coRQXBK9NxO98XUv3ZD6AK3xzHCxV6+b7lrquKwaKzA= +k8s.io/utils v0.0.0-20240902221715-702e33fdd3c3 h1:b2FmK8YH+QEwq/Sy2uAEhmqL5nPfGYbJOcaqjeYYZoA= +k8s.io/utils v0.0.0-20240902221715-702e33fdd3c3/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0= rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8= rsc.io/quote/v3 v3.1.0/go.mod h1:yEA65RcK8LyAZtP9Kv3t0HmxON59tX3rD+tICJqUlj0= rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA= diff --git a/hack/update-go-mod/go.mod b/hack/update-go-mod/go.mod index dbd063dbd..4d27e88ff 100644 --- a/hack/update-go-mod/go.mod +++ b/hack/update-go-mod/go.mod @@ -4,4 +4,4 @@ go 1.22.0 toolchain go1.23.0 -require golang.org/x/mod v0.20.0 +require golang.org/x/mod v0.21.0 diff --git a/hack/update-go-mod/go.sum b/hack/update-go-mod/go.sum index b34a2e966..da3e33ae2 100644 --- a/hack/update-go-mod/go.sum +++ b/hack/update-go-mod/go.sum @@ -1,2 +1,2 @@ -golang.org/x/mod v0.20.0 h1:utOm6MM3R3dnawAiJgn0y+xvuYRsm1RKM/4giyfDgV0= -golang.org/x/mod v0.20.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= +golang.org/x/mod v0.21.0 h1:vvrHzRwRfVKSiLrG+d4FMl/Qi4ukBCE6kZlTUkDYRT0= +golang.org/x/mod v0.21.0/go.mod h1:6SkKJ3Xj0I0BrPOZoBy3bdMptDDU9oJrpohJ3eWZ1fY= diff --git a/test/integration/concierge_whoami_test.go b/test/integration/concierge_whoami_test.go index a5c922452..ca7923748 100644 --- a/test/integration/concierge_whoami_test.go +++ b/test/integration/concierge_whoami_test.go @@ -7,8 +7,10 @@ import ( "crypto/ecdsa" "crypto/elliptic" "crypto/rand" + "crypto/sha256" "crypto/x509" "crypto/x509/pkix" + "encoding/hex" "encoding/pem" "fmt" "testing" @@ -329,6 +331,13 @@ func TestWhoAmI_CSR_Parallel(t *testing.T) { csrConfig.CertData = crtPEM csrConfig.KeyData = keyPEM + bytesToHexOfSHA256 := func(b []byte) string { + shasum := sha256.Sum256(b) + return hex.EncodeToString(shasum[:]) + } + + t.Logf("sha256 of crtPEM: %s", bytesToHexOfSHA256(crtPEM)) + whoAmI, err := testlib.NewKubeclient(t, csrConfig).PinnipedConcierge.IdentityV1alpha1().WhoAmIRequests(). Create(ctx, &identityv1alpha1.WhoAmIRequest{}, metav1.CreateOptions{}) require.NoError(t, err, testlib.Sdump(err)) From 08abff1cae2384e45986836fbb5f0b818941fd28 Mon Sep 17 00:00:00 2001 From: Joshua Casey Date: Tue, 3 Sep 2024 07:34:30 -0500 Subject: [PATCH 3/5] Bump golanglint-ci to 1.60.3 --- .golangci.yaml | 2 -- cmd/pinniped/cmd/kubeconfig.go | 2 +- hack/lib/lint-version.txt | 2 +- .../oidcclientwatcher/oidc_client_watcher.go | 1 + internal/endpointaddr/endpointaddr.go | 5 ++++- .../endpoints/chooseidp/chooseidphtml/chooseidphtml.go | 6 +++--- .../endpoints/login/loginhtml/loginhtml.go | 4 ++-- internal/federationdomain/formposthtml/formposthtml.go | 6 +++--- internal/net/phttp/warning.go | 3 ++- internal/plog/config.go | 1 + internal/plog/level.go | 3 ++- internal/testutil/totp/totp.go | 2 +- internal/upstreamoidc/upstreamoidc_test.go | 2 +- pkg/oidcclient/login.go | 2 +- test/integration/limited_ciphers_utils_test.go | 6 +++--- test/integration/main_test.go | 2 +- test/integration/pod_shutdown_test.go | 10 +++++----- 17 files changed, 32 insertions(+), 27 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index 17d5a317b..17f6903bc 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -20,7 +20,6 @@ linters: # - depguard - dogsled - exhaustive - - exportloopref - funlen - gochecknoglobals - gochecknoinits @@ -39,7 +38,6 @@ linters: - nolintlint - prealloc - rowserrcheck - - exportloopref - sqlclosecheck - unconvert - whitespace diff --git a/cmd/pinniped/cmd/kubeconfig.go b/cmd/pinniped/cmd/kubeconfig.go index defe624ca..8e536983c 100644 --- a/cmd/pinniped/cmd/kubeconfig.go +++ b/cmd/pinniped/cmd/kubeconfig.go @@ -745,7 +745,7 @@ func validateKubeconfig(ctx context.Context, flags getKubeconfigParams, kubeconf func countCACerts(pemData []byte) int { pool := x509.NewCertPool() pool.AppendCertsFromPEM(pemData) - return len(pool.Subjects()) + return len(pool.Subjects()) //nolint:staticcheck // there's no other clear way to mimic this legacy behavior } func hasPendingStrategy(credentialIssuer *conciergeconfigv1alpha1.CredentialIssuer) bool { diff --git a/hack/lib/lint-version.txt b/hack/lib/lint-version.txt index f6c9d766c..8f16209d1 100644 --- a/hack/lib/lint-version.txt +++ b/hack/lib/lint-version.txt @@ -1 +1 @@ -1.60.1 +1.60.3 diff --git a/internal/controller/supervisorconfig/oidcclientwatcher/oidc_client_watcher.go b/internal/controller/supervisorconfig/oidcclientwatcher/oidc_client_watcher.go index a9573ba42..c57a1c6e5 100644 --- a/internal/controller/supervisorconfig/oidcclientwatcher/oidc_client_watcher.go +++ b/internal/controller/supervisorconfig/oidcclientwatcher/oidc_client_watcher.go @@ -146,6 +146,7 @@ func (c *oidcClientWatcherController) updateStatus( updated.Status.Phase = supervisorconfigv1alpha1.OIDCClientPhaseError } + //nolint:gosec // looks like we are willing to accept the risk that there are less than 2147483647 dynamic clients updated.Status.TotalClientSecrets = int32(totalClientSecrets) if equality.Semantic.DeepEqual(upstream, updated) { diff --git a/internal/endpointaddr/endpointaddr.go b/internal/endpointaddr/endpointaddr.go index d6c76be4e..1307d5054 100644 --- a/internal/endpointaddr/endpointaddr.go +++ b/internal/endpointaddr/endpointaddr.go @@ -71,7 +71,10 @@ func Parse(endpoint string, defaultPort uint16) (HostPort, error) { return HostPort{}, fmt.Errorf("host %q is not a valid hostname or IP address", host) } - return HostPort{Host: host, Port: uint16(integerPort)}, nil + return HostPort{ + Host: host, + Port: uint16(integerPort), //nolint:gosec // this cast is checked by validation.IsValidPortNum above + }, nil } // ParseFromURL wraps Parse but specifically takes a url.URL instead of an endpoint string. diff --git a/internal/federationdomain/endpoints/chooseidp/chooseidphtml/chooseidphtml.go b/internal/federationdomain/endpoints/chooseidp/chooseidphtml/chooseidphtml.go index e45e1bf38..b07dc6f50 100644 --- a/internal/federationdomain/endpoints/chooseidp/chooseidphtml/chooseidphtml.go +++ b/internal/federationdomain/endpoints/chooseidp/chooseidphtml/chooseidphtml.go @@ -1,4 +1,4 @@ -// Copyright 2023 the Pinniped contributors. All Rights Reserved. +// Copyright 2023-2024 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package chooseidphtml @@ -28,8 +28,8 @@ var ( // Parse the Go templated HTML and inject functions providing the minified inline CSS and JS. parsedHTMLTemplate = template.Must(template.New("choose_idp.gohtml").Funcs(template.FuncMap{ - "minifiedCSS": func() template.CSS { return template.CSS(CSS()) }, - "minifiedJS": func() template.JS { return template.JS(JS()) }, //nolint:gosec // This is 100% static input, not attacker-controlled. + "minifiedCSS": func() template.CSS { return template.CSS(CSS()) }, //nolint:gosec // This is 100% static input, not attacker-controlled. + "minifiedJS": func() template.JS { return template.JS(JS()) }, //nolint:gosec // This is 100% static input, not attacker-controlled. }).Parse(rawHTMLTemplate)) // Generate the CSP header value once since it's effectively constant. diff --git a/internal/federationdomain/endpoints/login/loginhtml/loginhtml.go b/internal/federationdomain/endpoints/login/loginhtml/loginhtml.go index c09b5ca84..1de88a9da 100644 --- a/internal/federationdomain/endpoints/login/loginhtml/loginhtml.go +++ b/internal/federationdomain/endpoints/login/loginhtml/loginhtml.go @@ -1,4 +1,4 @@ -// Copyright 2022-2023 the Pinniped contributors. All Rights Reserved. +// Copyright 2022-2024 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 // Package loginhtml defines HTML templates used by the Supervisor. @@ -25,7 +25,7 @@ var ( // Parse the Go templated HTML and inject functions providing the minified inline CSS and JS. parsedHTMLTemplate = template.Must(template.New("login_form.gohtml").Funcs(template.FuncMap{ - "minifiedCSS": func() template.CSS { return template.CSS(CSS()) }, + "minifiedCSS": func() template.CSS { return template.CSS(CSS()) }, //nolint:gosec // This is 100% static input, not attacker-controlled. }).Parse(rawHTMLTemplate)) // Generate the CSP header value once since it's effectively constant. diff --git a/internal/federationdomain/formposthtml/formposthtml.go b/internal/federationdomain/formposthtml/formposthtml.go index cdf2b85b6..062a3efe1 100644 --- a/internal/federationdomain/formposthtml/formposthtml.go +++ b/internal/federationdomain/formposthtml/formposthtml.go @@ -1,4 +1,4 @@ -// Copyright 2021-2023 the Pinniped contributors. All Rights Reserved. +// Copyright 2021-2024 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 // Package formposthtml defines HTML templates used by the Supervisor. @@ -29,8 +29,8 @@ var ( // Parse the Go templated HTML and inject functions providing the minified inline CSS and JS. parsedHTMLTemplate = template.Must(template.New("form_post.gohtml").Funcs(template.FuncMap{ - "minifiedCSS": func() template.CSS { return template.CSS(minifiedCSS) }, - "minifiedJS": func() template.JS { return template.JS(minifiedJS) }, //nolint:gosec // This is 100% static input, not attacker-controlled. + "minifiedCSS": func() template.CSS { return template.CSS(minifiedCSS) }, //nolint:gosec // This is 100% static input, not attacker-controlled. + "minifiedJS": func() template.JS { return template.JS(minifiedJS) }, //nolint:gosec // This is 100% static input, not attacker-controlled. }).Parse(rawHTMLTemplate)) // Generate the CSP header value once since it's effectively constant. diff --git a/internal/net/phttp/warning.go b/internal/net/phttp/warning.go index 99359461f..5cde35102 100644 --- a/internal/net/phttp/warning.go +++ b/internal/net/phttp/warning.go @@ -1,4 +1,4 @@ -// Copyright 2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2021-2024 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package phttp @@ -40,6 +40,7 @@ func getWarningHandler() rest.WarningHandler { // the client-go rest.WarningHandlers all log warnings with non-empty message and code=299, agent is ignored // no deduplication or color output when running from a non-terminal such as a pod + //nolint:gosec // this is an int, cast to uintptr, cast back to int if isTerm := term.IsTerminal(int(os.Stderr.Fd())); !isTerm { return rest.WarningLogger{} } diff --git a/internal/plog/config.go b/internal/plog/config.go index b2c60416f..ae3686da5 100644 --- a/internal/plog/config.go +++ b/internal/plog/config.go @@ -57,6 +57,7 @@ func ValidateAndSetLogLevelAndFormatGlobally(ctx context.Context, spec LogSpec) if _, err := logs.GlogSetter(strconv.Itoa(int(klogLevel))); err != nil { panic(err) // programmer error } + //nolint:gosec // the range for klogLevel is [0,108] globalLevel.SetLevel(zapcore.Level(-klogLevel)) // klog levels are inverted when zap handles them var encoding string diff --git a/internal/plog/level.go b/internal/plog/level.go index 4daa7b936..56eb8483f 100644 --- a/internal/plog/level.go +++ b/internal/plog/level.go @@ -1,4 +1,4 @@ -// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package plog @@ -45,6 +45,7 @@ func Enabled(level LogLevel) bool { l := klogLevelForPlogLevel(level) // check that both our global level and the klog global level agree that the plog level is enabled // klog levels are inverted when zap handles them + //nolint:gosec // the range for klogLevel is [0,108] return globalLevel.Enabled(zapcore.Level(-l)) && klog.V(l).Enabled() } diff --git a/internal/testutil/totp/totp.go b/internal/testutil/totp/totp.go index 7e2c153cf..9180dad6f 100644 --- a/internal/testutil/totp/totp.go +++ b/internal/testutil/totp/totp.go @@ -80,7 +80,7 @@ func GenerateOTPCode(t *testing.T, token string, when time.Time) (string, int64) ((int(sum[offset+2] & mask3)) << shift8) | (int(sum[offset+3]) & mask3)) - modulo := int32(value % int64(math.Pow10(length))) + modulo := int32(value % int64(math.Pow10(length))) //nolint:gosec // the resulting number must be less than 10^6 format := fmt.Sprintf("%%0%dd", length) diff --git a/internal/upstreamoidc/upstreamoidc_test.go b/internal/upstreamoidc/upstreamoidc_test.go index 762e09de2..fbbe156bb 100644 --- a/internal/upstreamoidc/upstreamoidc_test.go +++ b/internal/upstreamoidc/upstreamoidc_test.go @@ -1461,7 +1461,7 @@ func forceUserInfoWithClaims(subject string, claims string) *coreosoidc.UserInfo // this is some dark magic to set a private field claimsField := reflect.ValueOf(userInfo).Elem().FieldByName("claims") - claimsPointer := (*[]byte)(unsafe.Pointer(claimsField.UnsafeAddr())) + claimsPointer := (*[]byte)(unsafe.Pointer(claimsField.UnsafeAddr())) //nolint:gosec // this is a test hack we are willing to live with *claimsPointer = []byte(claims) return userInfo diff --git a/pkg/oidcclient/login.go b/pkg/oidcclient/login.go index f1b7e6d71..53267d6c2 100644 --- a/pkg/oidcclient/login.go +++ b/pkg/oidcclient/login.go @@ -75,7 +75,7 @@ const ( ) // stdin returns the file descriptor for stdin as an int. -func stdin() int { return int(os.Stdin.Fd()) } +func stdin() int { return int(os.Stdin.Fd()) } //nolint:gosec // this is an int, cast to uintptr, cast back to int type handlerState struct { // Basic parameters. diff --git a/test/integration/limited_ciphers_utils_test.go b/test/integration/limited_ciphers_utils_test.go index 51f52181d..f2eef58cf 100644 --- a/test/integration/limited_ciphers_utils_test.go +++ b/test/integration/limited_ciphers_utils_test.go @@ -114,7 +114,7 @@ func expectTLSConfigForServicePort( ) { portAsInt, err := strconv.Atoi(localPortAsStr) require.NoError(t, err) - portAsUint := uint16(portAsInt) // okay to cast because it will only be legal port numbers + portAsUint := uint16(portAsInt) //nolint:gosec // okay to cast because it will only be legal port numbers startKubectlPortForward(ctx, t, localPortAsStr, "443", serviceName, serviceNamespace) @@ -193,7 +193,7 @@ func restartAllPodsOfApp( // Scale down the deployment's number of replicas to 0, which will shut down all the pods. originalScale := updateDeploymentScale(t, namespace, appName, 0) - require.Greater(t, originalScale, 0) + require.Greater(t, int(originalScale), 0) testlib.RequireEventually(t, func(requireEventually *require.Assertions) { newPods := getRunningPodsByNamePrefix(t, namespace, appName+"-", ignorePodsWithNameSubstring) @@ -205,7 +205,7 @@ func restartAllPodsOfApp( testlib.RequireEventually(t, func(requireEventually *require.Assertions) { newPods := getRunningPodsByNamePrefix(t, namespace, appName+"-", ignorePodsWithNameSubstring) - requireEventually.Len(newPods, originalScale, "wanted %d pods", originalScale) + requireEventually.Equal(len(newPods), int(originalScale), "wanted %d pods", originalScale) requireEventually.True(allPodsReady(newPods), "wanted all new pods to be ready") }, 2*time.Minute, 200*time.Millisecond) } diff --git a/test/integration/main_test.go b/test/integration/main_test.go index f3b5992aa..d28e3527b 100644 --- a/test/integration/main_test.go +++ b/test/integration/main_test.go @@ -28,7 +28,7 @@ func TestMain(m *testing.M) { func splitIntegrationTestsIntoBuckets(m *testing.M) { // this is some dark magic to set a private field testsField := reflect.ValueOf(m).Elem().FieldByName("tests") - testsPointer := (*[]testing.InternalTest)(unsafe.Pointer(testsField.UnsafeAddr())) + testsPointer := (*[]testing.InternalTest)(unsafe.Pointer(testsField.UnsafeAddr())) //nolint:gosec // this is a test hack we are willing to live with tests := *testsPointer diff --git a/test/integration/pod_shutdown_test.go b/test/integration/pod_shutdown_test.go index a366d6dd1..5931e3696 100644 --- a/test/integration/pod_shutdown_test.go +++ b/test/integration/pod_shutdown_test.go @@ -97,7 +97,7 @@ func shutdownAllPodsOfApp( var newPods []corev1.Pod testlib.RequireEventually(t, func(requireEventually *require.Assertions) { newPods = getRunningPodsByNamePrefix(t, namespace, appName+"-", ignorePodsWithNameSubstring) - requireEventually.Len(newPods, originalScale, "wanted pods to return to original scale") + requireEventually.Equal(len(newPods), int(originalScale), "wanted pods to return to original scale") requireEventually.True(allPodsReady(newPods), "wanted all new pods to be ready") }, 2*time.Minute, 200*time.Millisecond) @@ -111,7 +111,7 @@ func shutdownAllPodsOfApp( // Double check: the deployment's previous scale should have equaled the actual number of running pods from // the start of the test (before we scaled down). - require.Equal(t, len(initialPods), originalScale) + require.Equal(t, len(initialPods), int(originalScale)) // Now that we have adjusted the scale to 0, the pods should go away. // Our pods are intended to gracefully shut down within a few seconds, so fail unless it happens fairly quickly. @@ -204,7 +204,7 @@ func isPodReady(pod corev1.Pod) bool { return false } -func updateDeploymentScale(t *testing.T, namespace string, deploymentName string, newScale int) int { +func updateDeploymentScale(t *testing.T, namespace string, deploymentName string, newScale int32) int32 { t.Helper() ctx, cancel := context.WithTimeout(context.Background(), 1*time.Minute) defer cancel() @@ -214,13 +214,13 @@ func updateDeploymentScale(t *testing.T, namespace string, deploymentName string require.NoError(t, err) desiredScale := initialScale.DeepCopy() - desiredScale.Spec.Replicas = int32(newScale) + desiredScale.Spec.Replicas = newScale updatedScale, err := client.AppsV1().Deployments(namespace).UpdateScale(ctx, deploymentName, desiredScale, metav1.UpdateOptions{}) require.NoError(t, err) t.Logf("updated scale of Deployment %s/%s from %d to %d", namespace, deploymentName, initialScale.Spec.Replicas, updatedScale.Spec.Replicas) - return int(initialScale.Spec.Replicas) + return initialScale.Spec.Replicas } func tailFollowPodLogs(t *testing.T, pod corev1.Pod) (chan struct{}, *bytes.Buffer) { From 72fa369fc92f7b2b5f61aced7e9826909f113ff2 Mon Sep 17 00:00:00 2001 From: Joshua Casey Date: Wed, 4 Sep 2024 20:48:44 -0500 Subject: [PATCH 4/5] Integration tests should use PINNIPED_TEST_SUPERVISOR_SERVICE_NAME to decide where to port-forward --- test/integration/securetls_test.go | 19 +++++++++++++------ test/testlib/env.go | 2 ++ 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/test/integration/securetls_test.go b/test/integration/securetls_test.go index 7daf2ba3a..7e39d46e1 100644 --- a/test/integration/securetls_test.go +++ b/test/integration/securetls_test.go @@ -118,15 +118,22 @@ func TestSecureTLSSupervisor(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) t.Cleanup(cancel) - supervisorIssuer := testlib.NewSupervisorIssuer(t, env.SupervisorHTTPSAddress) + // On kind, this will generally be app-name-nodeport + // On GKE and AKS, this will generally be app-name-loadbalancer (their ingresses use IP addresses) + // On EKS, this will generally be app-name-loadbalancer (this ingress uses DNS addresses), but we should rely on + // the PINNIPED_TEST_SUPERVISOR_SERVICE_NAME variable. + serviceName := env.SupervisorServiceName + if serviceName == "" { + serviceName = env.SupervisorAppName + "-nodeport" - serviceSuffix := "-nodeport" - if supervisorIssuer.IsIPAddress() { - // Then there's no nodeport service to connect to, it's a load balancer service! - serviceSuffix = "-loadbalancer" + supervisorIssuer := testlib.NewSupervisorIssuer(t, env.SupervisorHTTPSAddress) + if supervisorIssuer.IsIPAddress() { + // Then there's no nodeport service to connect to, it's a load balancer service! + serviceName = env.SupervisorAppName + "-loadbalancer" + } } - startKubectlPortForward(ctx, t, "10448", "443", env.SupervisorAppName+serviceSuffix, env.SupervisorNamespace) + startKubectlPortForward(ctx, t, "10448", "443", serviceName, env.SupervisorNamespace) stdout, stderr := testlib.RunNmapSSLEnum(t, "127.0.0.1", 10448) diff --git a/test/testlib/env.go b/test/testlib/env.go index 68d1c8e5c..9e3c1a096 100644 --- a/test/testlib/env.go +++ b/test/testlib/env.go @@ -44,6 +44,7 @@ type TestEnv struct { SupervisorNamespace string `json:"supervisorNamespace"` ConciergeAppName string `json:"conciergeAppName"` SupervisorAppName string `json:"supervisorAppName"` + SupervisorServiceName string `json:"supervisorServiceName"` SupervisorCustomLabels map[string]string `json:"supervisorCustomLabels"` ConciergeCustomLabels map[string]string `json:"conciergeCustomLabels"` KubernetesDistribution KubeDistro `json:"kubernetesDistribution"` @@ -259,6 +260,7 @@ func loadEnvVars(t *testing.T, result *TestEnv) { result.TestWebhook.Endpoint = needEnv(t, "PINNIPED_TEST_WEBHOOK_ENDPOINT") result.SupervisorNamespace = needEnv(t, "PINNIPED_TEST_SUPERVISOR_NAMESPACE") result.SupervisorAppName = needEnv(t, "PINNIPED_TEST_SUPERVISOR_APP_NAME") + result.SupervisorServiceName = os.Getenv("PINNIPED_TEST_SUPERVISOR_SERVICE_NAME") result.TestWebhook.TLS = &authenticationv1alpha1.TLSSpec{CertificateAuthorityData: needEnv(t, "PINNIPED_TEST_WEBHOOK_CA_BUNDLE")} result.SupervisorHTTPSIngressAddress = os.Getenv("PINNIPED_TEST_SUPERVISOR_HTTPS_INGRESS_ADDRESS") From 00e9b347dbe4c257182dd768b6cbf09a842e10e8 Mon Sep 17 00:00:00 2001 From: Joshua Casey Date: Thu, 5 Sep 2024 08:16:36 -0500 Subject: [PATCH 5/5] Bump BUILD_IMAGE in Dockerfiles --- Dockerfile | 2 +- hack/Dockerfile_fips | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Dockerfile b/Dockerfile index bc06fffbf..8e85acbe9 100644 --- a/Dockerfile +++ b/Dockerfile @@ -3,7 +3,7 @@ # Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. # SPDX-License-Identifier: Apache-2.0 -ARG BUILD_IMAGE=golang:1.23.0@sha256:613a108a4a4b1dfb6923305db791a19d088f77632317cfc3446825c54fb862cd +ARG BUILD_IMAGE=golang:1.23.0@sha256:1a6db32ea47a4910759d5bcbabeb8a8b42658e311bd8348ea4763735447c636c ARG BASE_IMAGE=gcr.io/distroless/static:nonroot@sha256:8dd8d3ca2cf283383304fd45a5c9c74d5f2cd9da8d3b077d720e264880077c65 # Prepare to cross-compile by always running the build stage in the build platform, not the target platform. diff --git a/hack/Dockerfile_fips b/hack/Dockerfile_fips index 93d40861b..51e3c9f1d 100644 --- a/hack/Dockerfile_fips +++ b/hack/Dockerfile_fips @@ -16,7 +16,7 @@ # See https://go.googlesource.com/go/+/dev.boringcrypto/README.boringcrypto.md # and https://kupczynski.info/posts/fips-golang/ for details. -ARG BUILD_IMAGE=golang:1.23.0@sha256:613a108a4a4b1dfb6923305db791a19d088f77632317cfc3446825c54fb862cd +ARG BUILD_IMAGE=golang:1.23.0@sha256:1a6db32ea47a4910759d5bcbabeb8a8b42658e311bd8348ea4763735447c636c ARG BASE_IMAGE=gcr.io/distroless/static:nonroot@sha256:8dd8d3ca2cf283383304fd45a5c9c74d5f2cd9da8d3b077d720e264880077c65 # This is not currently using --platform to prepare to cross-compile because we use gcc below to build