From e69ff70fc52270031266f0b053c1a071282df630 Mon Sep 17 00:00:00 2001 From: Mile Druzijanic Date: Thu, 4 Jan 2024 22:45:37 +0100 Subject: [PATCH 1/4] removing Roots deprecated Subjects field in tests --- credentials/tls/certprovider/pemfile/watcher_test.go | 4 +--- credentials/tls/certprovider/store_test.go | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/credentials/tls/certprovider/pemfile/watcher_test.go b/credentials/tls/certprovider/pemfile/watcher_test.go index 521f762d3a41..c8fc87aa06c2 100644 --- a/credentials/tls/certprovider/pemfile/watcher_test.go +++ b/credentials/tls/certprovider/pemfile/watcher_test.go @@ -26,8 +26,6 @@ import ( "testing" "time" - "github.com/google/go-cmp/cmp" - "github.com/google/go-cmp/cmp/cmpopts" "google.golang.org/grpc/credentials/tls/certprovider" "google.golang.org/grpc/internal/grpctest" "google.golang.org/grpc/internal/testutils" @@ -69,7 +67,7 @@ func compareKeyMaterial(got, want *certprovider.KeyMaterial) error { // library does not provide a way to compare CertPool values. Comparing the // subjects field of the certs in the CertPool seems like a reasonable // approach. - if gotR, wantR := got.Roots.Subjects(), want.Roots.Subjects(); !cmp.Equal(gotR, wantR, cmpopts.EquateEmpty()) { + if gotR, wantR := got.Roots, want.Roots; !gotR.Equal(wantR) { return fmt.Errorf("keyMaterial roots = %v, want %v", gotR, wantR) } return nil diff --git a/credentials/tls/certprovider/store_test.go b/credentials/tls/certprovider/store_test.go index dce61912977b..a20cf6768025 100644 --- a/credentials/tls/certprovider/store_test.go +++ b/credentials/tls/certprovider/store_test.go @@ -28,8 +28,6 @@ import ( "testing" "time" - "github.com/google/go-cmp/cmp" - "github.com/google/go-cmp/cmp/cmpopts" "google.golang.org/grpc/internal/grpctest" "google.golang.org/grpc/internal/testutils" "google.golang.org/grpc/testdata" @@ -168,7 +166,7 @@ func compareKeyMaterial(got, want *KeyMaterial) error { // library does not provide a way to compare CertPool values. Comparing the // subjects field of the certs in the CertPool seems like a reasonable // approach. - if gotR, wantR := got.Roots.Subjects(), want.Roots.Subjects(); !cmp.Equal(gotR, wantR, cmpopts.EquateEmpty()) { + if gotR, wantR := got.Roots, want.Roots; !gotR.Equal(wantR) { return fmt.Errorf("keyMaterial roots = %v, want %v", gotR, wantR) } return nil From 5d5b9aa3a08c519b8f333e2f65942154e97d13a0 Mon Sep 17 00:00:00 2001 From: Mile Druzijanic Date: Mon, 8 Jan 2024 08:26:14 +0100 Subject: [PATCH 2/4] clean up of unneeded comments --- credentials/tls/certprovider/pemfile/watcher_test.go | 6 ------ credentials/tls/certprovider/store_test.go | 6 ------ 2 files changed, 12 deletions(-) diff --git a/credentials/tls/certprovider/pemfile/watcher_test.go b/credentials/tls/certprovider/pemfile/watcher_test.go index c8fc87aa06c2..486bc881a64e 100644 --- a/credentials/tls/certprovider/pemfile/watcher_test.go +++ b/credentials/tls/certprovider/pemfile/watcher_test.go @@ -61,12 +61,6 @@ func compareKeyMaterial(got, want *certprovider.KeyMaterial) error { } } - // x509.CertPool contains only unexported fields some of which contain other - // unexported fields. So usage of cmp.AllowUnexported() or - // cmpopts.IgnoreUnexported() does not help us much here. Also, the standard - // library does not provide a way to compare CertPool values. Comparing the - // subjects field of the certs in the CertPool seems like a reasonable - // approach. if gotR, wantR := got.Roots, want.Roots; !gotR.Equal(wantR) { return fmt.Errorf("keyMaterial roots = %v, want %v", gotR, wantR) } diff --git a/credentials/tls/certprovider/store_test.go b/credentials/tls/certprovider/store_test.go index a20cf6768025..13eda4fc8b94 100644 --- a/credentials/tls/certprovider/store_test.go +++ b/credentials/tls/certprovider/store_test.go @@ -160,12 +160,6 @@ func compareKeyMaterial(got, want *KeyMaterial) error { } } - // x509.CertPool contains only unexported fields some of which contain other - // unexported fields. So usage of cmp.AllowUnexported() or - // cmpopts.IgnoreUnexported() does not help us much here. Also, the standard - // library does not provide a way to compare CertPool values. Comparing the - // subjects field of the certs in the CertPool seems like a reasonable - // approach. if gotR, wantR := got.Roots, want.Roots; !gotR.Equal(wantR) { return fmt.Errorf("keyMaterial roots = %v, want %v", gotR, wantR) } From ef10507673682f0d33cafb9c2b575eeed5ffc2a8 Mon Sep 17 00:00:00 2001 From: Mile Druzijanic Date: Wed, 10 Jan 2024 01:17:17 +0100 Subject: [PATCH 3/4] vet: remove ignore of Roots.Subjects --- credentials/tls/certprovider/pemfile/watcher_test.go | 1 + credentials/tls/certprovider/store_test.go | 1 + vet.sh | 2 -- 3 files changed, 2 insertions(+), 2 deletions(-) diff --git a/credentials/tls/certprovider/pemfile/watcher_test.go b/credentials/tls/certprovider/pemfile/watcher_test.go index 486bc881a64e..ee653a856d77 100644 --- a/credentials/tls/certprovider/pemfile/watcher_test.go +++ b/credentials/tls/certprovider/pemfile/watcher_test.go @@ -64,6 +64,7 @@ func compareKeyMaterial(got, want *certprovider.KeyMaterial) error { if gotR, wantR := got.Roots, want.Roots; !gotR.Equal(wantR) { return fmt.Errorf("keyMaterial roots = %v, want %v", gotR, wantR) } + return nil } diff --git a/credentials/tls/certprovider/store_test.go b/credentials/tls/certprovider/store_test.go index 13eda4fc8b94..428bdf53bab1 100644 --- a/credentials/tls/certprovider/store_test.go +++ b/credentials/tls/certprovider/store_test.go @@ -163,6 +163,7 @@ func compareKeyMaterial(got, want *KeyMaterial) error { if gotR, wantR := got.Roots, want.Roots; !gotR.Equal(wantR) { return fmt.Errorf("keyMaterial roots = %v, want %v", gotR, wantR) } + return nil } diff --git a/vet.sh b/vet.sh index a4990e058336..5da38a40996f 100755 --- a/vet.sh +++ b/vet.sh @@ -185,8 +185,6 @@ GetSafeRegexMatch GetSuffixMatch GetTlsCertificateCertificateProviderInstance GetValidationContextCertificateProviderInstance -XXXXX TODO: Remove the below deprecation usages: -Roots.Subjects XXXXX PleaseIgnoreUnused' echo SUCCESS From b11a0bdb4a6a75b0f48b96b0c592b62d27451303 Mon Sep 17 00:00:00 2001 From: Mile Druzijanic Date: Fri, 12 Jan 2024 15:43:21 +0100 Subject: [PATCH 4/4] quick fmt update --- credentials/tls/certprovider/pemfile/watcher_test.go | 2 +- credentials/tls/certprovider/store_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/credentials/tls/certprovider/pemfile/watcher_test.go b/credentials/tls/certprovider/pemfile/watcher_test.go index ee653a856d77..3f015833f2c5 100644 --- a/credentials/tls/certprovider/pemfile/watcher_test.go +++ b/credentials/tls/certprovider/pemfile/watcher_test.go @@ -64,7 +64,7 @@ func compareKeyMaterial(got, want *certprovider.KeyMaterial) error { if gotR, wantR := got.Roots, want.Roots; !gotR.Equal(wantR) { return fmt.Errorf("keyMaterial roots = %v, want %v", gotR, wantR) } - + return nil } diff --git a/credentials/tls/certprovider/store_test.go b/credentials/tls/certprovider/store_test.go index 428bdf53bab1..d6f673b85713 100644 --- a/credentials/tls/certprovider/store_test.go +++ b/credentials/tls/certprovider/store_test.go @@ -163,7 +163,7 @@ func compareKeyMaterial(got, want *KeyMaterial) error { if gotR, wantR := got.Roots, want.Roots; !gotR.Equal(wantR) { return fmt.Errorf("keyMaterial roots = %v, want %v", gotR, wantR) } - + return nil }