Refactor tlsconfigutil.CABundle 'getters' to not have 'get' in the name

Co-authored-by: Ryan Richard <richardry@vmware.com>
This commit is contained in:
Joshua Casey
2024-07-26 11:11:03 -05:00
committed by Ryan Richard
parent 4cf0e46c38
commit fcceeed9fa
8 changed files with 43 additions and 43 deletions

View File

@@ -237,7 +237,7 @@ func (c *jwtCacheFillerController) syncIndividualJWTAuthenticator(ctx context.Co
if jwtAuthenticatorFromCache != nil &&
reflect.DeepEqual(jwtAuthenticatorFromCache.spec, &jwtAuthenticator.Spec) &&
tlsBundleOk && // if there was any error while validating the CA bundle, then run remaining validations and update status
jwtAuthenticatorFromCache.caBundlePEMSHA256 == caBundle.GetCABundleHash() {
jwtAuthenticatorFromCache.caBundlePEMSHA256 == caBundle.Hash() {
c.log.WithValues("jwtAuthenticator", klog.KObj(jwtAuthenticator), "issuer", jwtAuthenticator.Spec.Issuer).
Info("actual jwt authenticator and desired jwt authenticator are the same")
// Stop, no more work to be done. This authenticator is already validated and cached.
@@ -249,7 +249,7 @@ func (c *jwtCacheFillerController) syncIndividualJWTAuthenticator(ctx context.Co
_, conditions, issuerOk := c.validateIssuer(jwtAuthenticator.Spec.Issuer, conditions)
okSoFar := tlsBundleOk && issuerOk
client := phttp.Default(caBundle.GetCertPool())
client := phttp.Default(caBundle.CertPool())
client.Timeout = 30 * time.Second // copied from Kube OIDC code
coreOSCtx := coreosoidc.ClientContext(context.Background(), client)
@@ -269,7 +269,7 @@ func (c *jwtCacheFillerController) syncIndividualJWTAuthenticator(ctx context.Co
client,
jwtAuthenticator.Spec.DeepCopy(), // deep copy to avoid caching original object
keySet,
caBundle.GetCABundleHash(),
caBundle.Hash(),
conditions,
okSoFar)
errs = append(errs, err)

View File

@@ -162,7 +162,7 @@ func (c *webhookCacheFillerController) syncIndividualWebhookAuthenticator(ctx co
conditions := make([]*metav1.Condition, 0)
caBundle, conditions, tlsBundleOk := c.validateTLSBundle(webhookAuthenticator.Spec.TLS, conditions)
caBundlePEMSHA256 := caBundle.GetCABundleHash()
caBundlePEMSHA256 := caBundle.Hash()
// Only revalidate and update the cache if the cached authenticator is different from the desired authenticator.
// There is no need to repeat validations for a spec that was already successfully validated. We are making a
@@ -189,7 +189,7 @@ func (c *webhookCacheFillerController) syncIndividualWebhookAuthenticator(ctx co
endpointHostPort, conditions, endpointOk := c.validateEndpoint(webhookAuthenticator.Spec.Endpoint, conditions)
okSoFar := tlsBundleOk && endpointOk
conditions, tlsNegotiateErr := c.validateConnection(caBundle.GetCertPool(), endpointHostPort, conditions, okSoFar)
conditions, tlsNegotiateErr := c.validateConnection(caBundle.CertPool(), endpointHostPort, conditions, okSoFar)
errs = append(errs, tlsNegotiateErr)
okSoFar = okSoFar && tlsNegotiateErr == nil
@@ -197,7 +197,7 @@ func (c *webhookCacheFillerController) syncIndividualWebhookAuthenticator(ctx co
// Note that we use the whole URL when constructing the webhook client,
// not just the host and port that we validated above. We need the path, etc.
webhookAuthenticator.Spec.Endpoint,
caBundle.GetCABundle(),
caBundle.PEMBytes(),
conditions,
okSoFar,
)

View File

@@ -335,8 +335,8 @@ func (c *gitHubWatcherController) validateUpstreamAndUpdateConditions(ctx contro
githubConnectionCondition, hostURL, httpClient, githubConnectionErr := c.validateGitHubConnection(
hostPort,
caBundle.GetCABundle(),
caBundle.GetCertPool(),
caBundle.PEMBytes(),
caBundle.CertPool(),
hostCondition.Status == metav1.ConditionTrue,
tlsConfigCondition.Status == metav1.ConditionTrue,
)

View File

@@ -359,7 +359,7 @@ func (c *oidcWatcherController) validateIssuer(ctx context.Context, upstream *id
// Get the discovered provider and HTTP client from cache, if they are found in the cache.
cacheKey := oidcDiscoveryCacheKey{
issuer: upstream.Spec.Issuer,
caBundleHash: caBundle.GetCABundleHash(), // note that this will always return the same hash for nil input
caBundleHash: caBundle.Hash(), // note that this will always return the same hash for nil input
}
if cacheEntry := c.validatorCache.getProvider(cacheKey); cacheEntry != nil {
discoveredProvider = cacheEntry.provider
@@ -373,7 +373,7 @@ func (c *oidcWatcherController) validateIssuer(ctx context.Context, upstream *id
// If the provider does not exist in the cache, do a fresh discovery lookup and save to the cache.
if discoveredProvider == nil {
httpClient = defaultClientShortTimeout(caBundle.GetCertPool())
httpClient = defaultClientShortTimeout(caBundle.CertPool())
_, issuerURLCondition := validateHTTPSURL(upstream.Spec.Issuer, "issuer", reasonUnreachable)
if issuerURLCondition != nil {

View File

@@ -258,7 +258,7 @@ func ValidateGenericLDAP(
tlsSpec := tlsconfigutil.TLSSpecForSupervisor(upstream.Spec().TLSSpec())
tlsValidCondition, caBundle := tlsconfigutil.ValidateTLSConfig(tlsSpec, "spec.tls", upstream.Namespace(), secretInformer, configMapInformer)
conditions.Append(tlsValidCondition, true)
config.CABundle = caBundle.GetCABundle()
config.CABundle = caBundle.PEMBytes()
var ldapConnectionValidCondition, searchBaseFoundCondition *metav1.Condition
// No point in trying to connect to the server if the config was already determined to be invalid.

View File

@@ -24,32 +24,32 @@ func NewCABundle(caBundle []byte) (*CABundle, bool) {
}, certPool.AppendCertsFromPEM(caBundle)
}
// GetCABundle returns the CA certificate bundle PEM bytes.
func (c *CABundle) GetCABundle() []byte {
// PEMBytes returns the CA certificate bundle PEM bytes.
func (c *CABundle) PEMBytes() []byte {
if c == nil {
return nil
}
return c.caBundle
}
// GetCABundlePemString returns the certificate bundle PEM formatted as a string.
func (c *CABundle) GetCABundlePemString() string {
// PEMString returns the certificate bundle PEM formatted as a string.
func (c *CABundle) PEMString() string {
if c == nil {
return ""
}
return string(c.caBundle)
}
// GetCertPool returns a X509 cert pool with the CA certificate bundle.
func (c *CABundle) GetCertPool() *x509.CertPool {
// CertPool returns a X509 cert pool with the CA certificate bundle.
func (c *CABundle) CertPool() *x509.CertPool {
if c == nil {
return nil
}
return c.certPool
}
// GetCABundleHash returns a sha256 sum of the CA bundle bytes.
func (c *CABundle) GetCABundleHash() [32]byte {
// Hash returns a sha256 sum of the CA bundle bytes.
func (c *CABundle) Hash() [32]byte {
if c == nil || len(c.caBundle) < 1 {
return sHA256OfEmptyData
}
@@ -62,5 +62,5 @@ func (c *CABundle) GetCABundleHash() [32]byte {
// IsEqual returns whether a CABundle has the same CA certificate bundle as another.
func (c *CABundle) IsEqual(other *CABundle) bool {
return c.GetCABundleHash() == other.GetCABundleHash()
return c.Hash() == other.Hash()
}

View File

@@ -19,65 +19,65 @@ func TestNewCABundle(t *testing.T) {
caBundle, ok := NewCABundle(testCA.Bundle())
require.True(t, ok)
require.Equal(t, testCA.Bundle(), caBundle.GetCABundle())
require.Equal(t, sha256.Sum256(testCA.Bundle()), caBundle.GetCABundleHash())
require.Equal(t, string(testCA.Bundle()), caBundle.GetCABundlePemString())
require.True(t, testCA.Pool().Equal(caBundle.GetCertPool()), "should be the cert pool of the testCA")
require.Equal(t, testCA.Bundle(), caBundle.PEMBytes())
require.Equal(t, sha256.Sum256(testCA.Bundle()), caBundle.Hash())
require.Equal(t, string(testCA.Bundle()), caBundle.PEMString())
require.True(t, testCA.Pool().Equal(caBundle.CertPool()), "should be the cert pool of the testCA")
})
t.Run("returns false for non-certificate input", func(t *testing.T) {
caBundle, ok := NewCABundle([]byte("here are some bytes"))
require.False(t, ok)
require.Equal(t, []byte("here are some bytes"), caBundle.GetCABundle())
require.Equal(t, sha256.Sum256([]byte("here are some bytes")), caBundle.GetCABundleHash())
require.Equal(t, "here are some bytes", caBundle.GetCABundlePemString())
require.True(t, x509.NewCertPool().Equal(caBundle.GetCertPool()), "should be an empty cert pool")
require.Equal(t, []byte("here are some bytes"), caBundle.PEMBytes())
require.Equal(t, sha256.Sum256([]byte("here are some bytes")), caBundle.Hash())
require.Equal(t, "here are some bytes", caBundle.PEMString())
require.True(t, x509.NewCertPool().Equal(caBundle.CertPool()), "should be an empty cert pool")
})
}
func TestGetCABundle(t *testing.T) {
func TestPEMBytes(t *testing.T) {
t.Run("returns the CA bundle", func(t *testing.T) {
caBundle, _ := NewCABundle([]byte("here are some bytes"))
require.Equal(t, []byte("here are some bytes"), caBundle.GetCABundle())
require.Equal(t, []byte("here are some bytes"), caBundle.PEMBytes())
})
t.Run("handles nil receiver by returning nil", func(t *testing.T) {
var nilCABundle *CABundle
var expected []byte
require.Equal(t, expected, nilCABundle.GetCABundle())
require.Equal(t, expected, nilCABundle.PEMBytes())
})
}
func TestGetCABundlePemString(t *testing.T) {
func TestPEMString(t *testing.T) {
t.Run("returns the CA bundle PEM string", func(t *testing.T) {
caBundle, _ := NewCABundle([]byte("here is a string"))
require.Equal(t, "here is a string", caBundle.GetCABundlePemString())
require.Equal(t, "here is a string", caBundle.PEMString())
})
t.Run("handles nil receiver by returning empty sstring", func(t *testing.T) {
var nilCABundle *CABundle
require.Equal(t, "", nilCABundle.GetCABundlePemString())
require.Equal(t, "", nilCABundle.PEMString())
})
}
func TestGetCertPool(t *testing.T) {
func TestCertPool(t *testing.T) {
t.Run("returns the generated cert pool", func(t *testing.T) {
caBundle, _ := NewCABundle(nil)
require.Equal(t, x509.NewCertPool(), caBundle.GetCertPool())
require.Equal(t, x509.NewCertPool(), caBundle.CertPool())
})
t.Run("handles nil receiver by returning nil", func(t *testing.T) {
var nilCABundle *CABundle
var expected *x509.CertPool
require.Equal(t, expected, nilCABundle.GetCertPool())
require.Equal(t, expected, nilCABundle.CertPool())
})
}
func TestGetCABundleHash(t *testing.T) {
func TestHash(t *testing.T) {
sha256OfNil := [32]uint8{0xe3, 0xb0, 0xc4, 0x42, 0x98, 0xfc, 0x1c, 0x14, 0x9a, 0xfb, 0xf4, 0xc8, 0x99, 0x6f, 0xb9, 0x24, 0x27, 0xae, 0x41, 0xe4, 0x64, 0x9b, 0x93, 0x4c, 0xa4, 0x95, 0x99, 0x1b, 0x78, 0x52, 0xb8, 0x55}
// On the command line, `echo "test" | shasum -a 256` yields "9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08",
@@ -89,19 +89,19 @@ func TestGetCABundleHash(t *testing.T) {
t.Run("returns the SHA256", func(t *testing.T) {
caBundle, _ := NewCABundle([]byte("test"))
require.Equal(t, sha256OfTest, caBundle.GetCABundleHash())
require.Equal(t, sha256OfTest, caBundle.Hash())
})
t.Run("handles nil receiver by returning the hash of nil", func(t *testing.T) {
var nilCABundle *CABundle
require.Equal(t, sha256OfNil, nilCABundle.GetCABundleHash())
require.Equal(t, sha256OfNil, nilCABundle.Hash())
})
t.Run("handles improperly initialized receiver by returning the hash of nil", func(t *testing.T) {
caBundle := &CABundle{}
require.Equal(t, sha256OfNil, caBundle.GetCABundleHash())
require.Equal(t, sha256OfNil, caBundle.Hash())
})
t.Run("handles improperly initialized receiver by computing the hash", func(t *testing.T) {
@@ -109,7 +109,7 @@ func TestGetCABundleHash(t *testing.T) {
caBundle: []byte("test"),
}
require.Equal(t, sha256OfTest, caBundle.GetCABundleHash())
require.Equal(t, sha256OfTest, caBundle.Hash())
})
}

View File

@@ -100,7 +100,7 @@ func ValidateTLSConfig(
if err != nil {
return invalidTLSCondition(err.Error()), nil
}
if len(caBundle.GetCABundle()) < 1 {
if len(caBundle.PEMBytes()) < 1 {
// An empty or nil CA bundle results in a valid TLS condition which indicates that no CA data was supplied.
return validTLSCondition(fmt.Sprintf("%s is valid: %s", conditionPrefix, noTLSConfigurationMessage)), nil
}