Skip to content

Commit

Permalink
fix lint and unit
Browse files Browse the repository at this point in the history 
Signed-off-by: Mattia Lavacca <[email protected]>
  • Loading branch information
@mlavacca
mlavacca committed Dec 6, 2024
1 parent 2743445 commit c76772f
Show file tree
Hide file tree
Showing 8 changed files with 132 additions and 25 deletions.
2 changes: 1 addition & 1 deletion internal/controllers/gateway/backendtlspolicy_utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ func (r *BackendTLSPolicyReconciler) validateBackendTLSPolicy(ctx context.Contex
if invalidMessage != "" {
invalidMessage += " - "
}
invalidMessage = "SubjectAltNames feature is not currently supported"
invalidMessage += "SubjectAltNames feature is not currently supported"
}
if policy.Spec.Validation.WellKnownCACertificates != nil {
if invalidMessage != "" {
Expand Down
48 changes: 45 additions & 3 deletions internal/controllers/gateway/backendtlspolicy_utils_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -624,6 +624,41 @@ func TestValidateBackendTLSPolicy(t *testing.T) {
Message: "Multiple BackendTLSPolicies target the same service",
},
},
{
name: "policy with unsupported CACertificateRefs",
policy: &gatewayapi.BackendTLSPolicy{
ObjectMeta: metav1.ObjectMeta{
Name: "test-policy",
Namespace: "default",
},
Spec: gatewayapi.BackendTLSPolicySpec{
TargetRefs: []gatewayapi.LocalPolicyTargetReferenceWithSectionName{
{
LocalPolicyTargetReference: gatewayapi.LocalPolicyTargetReference{
Group: "core",
Kind: "Service",
Name: "example-service",
},
},
},
Validation: gatewayapi.BackendTLSPolicyValidation{
CACertificateRefs: []gatewayapi.LocalObjectReference{
{
Group: "core",
Kind: "Secret",
Name: "example-secret",
},
},
},
},
},
expected: &metav1.Condition{
Type: string(gatewayapi.PolicyConditionAccepted),
Status: metav1.ConditionFalse,
Reason: string(gatewayapi.PolicyReasonInvalid),
Message: "CACertificateRefs must reference ConfigMaps in the core group",
},
},
{
name: "policy with unsupported SubjectAltNames",
policy: &gatewayapi.BackendTLSPolicy{
Expand Down Expand Up @@ -655,7 +690,7 @@ func TestValidateBackendTLSPolicy(t *testing.T) {
Type: string(gatewayapi.PolicyConditionAccepted),
Status: metav1.ConditionFalse,
Reason: string(gatewayapi.PolicyReasonInvalid),
Message: "SubjectAltNames feature is not currently supported.",
Message: "SubjectAltNames feature is not currently supported",
},
},
{
Expand Down Expand Up @@ -684,7 +719,7 @@ func TestValidateBackendTLSPolicy(t *testing.T) {
Type: string(gatewayapi.PolicyConditionAccepted),
Status: metav1.ConditionFalse,
Reason: string(gatewayapi.PolicyReasonInvalid),
Message: "WellKnownCACertificates feature is not currently supported.",
Message: "WellKnownCACertificates feature is not currently supported",
},
},
{
Expand Down Expand Up @@ -712,14 +747,21 @@ func TestValidateBackendTLSPolicy(t *testing.T) {
},
},
WellKnownCACertificates: lo.ToPtr(gatewayapi.WellKnownCACertificatesType("well-known-ca")),
CACertificateRefs: []gatewayapi.LocalObjectReference{
{
Group: "core",
Kind: "Secret",
Name: "example-secret",
},
},
},
},
},
expected: &metav1.Condition{
Type: string(gatewayapi.PolicyConditionAccepted),
Status: metav1.ConditionFalse,
Reason: string(gatewayapi.PolicyReasonInvalid),
Message: "SubjectAltNames feature is not currently supported. WellKnownCACertificates feature is not currently supported.",
Message: "CACertificateRefs must reference ConfigMaps in the core group - SubjectAltNames feature is not currently supported - WellKnownCACertificates feature is not currently supported",
},
},
}
Expand Down
1 change: 1 addition & 0 deletions internal/dataplane/fallback/graph_dependencies.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ func ResolveDependencies(cache store.CacheStores, obj client.Object) ([]client.O
// Object types that have no dependencies.
case *netv1.IngressClass,
*corev1.Secret,
*corev1.ConfigMap,
*discoveryv1.EndpointSlice,
*gatewayapi.ReferenceGrant,
*gatewayapi.Gateway,
Expand Down
2 changes: 1 addition & 1 deletion internal/dataplane/translator/ingressrules.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ func (ir *ingressRules) handleServiceCACertificates(
)
continue
}
k.CACertificates = append(k.CACertificates, lo.ToPtr(string(certID)))
k.CACertificates = append(k.CACertificates, lo.ToPtr(certID))
}
}

Expand Down
21 changes: 10 additions & 11 deletions internal/dataplane/translator/translate_cacerts.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func (t *Translator) getCACerts() []kong.CACertificate {
for _, certSecret := range caCertSecrets {
idBytes, ok := certSecret.Data["id"]
if !ok {
t.registerTranslationFailure("invalid CA certificate: missing 'id' field in data", certSecret)
t.registerTranslationFailure("invalid secret CA certificate: missing 'id' field in data", certSecret)
continue
}
secretID := string(idBytes)
Expand All @@ -41,7 +41,7 @@ func (t *Translator) getCACerts() []kong.CACertificate {
if !certExists {
relatedObjects := getPluginsAssociatedWithCACertSecret(secretID, t.storer)
relatedObjects = append(relatedObjects, certSecret.DeepCopy())
t.registerTranslationFailure(fmt.Sprintf("invalid CA certificate: %s", err), relatedObjects...)
t.registerTranslationFailure(fmt.Sprintf(`invalid secret CA certificate %s/%s, neither "cert" nor "ca.crt" key exist`, certSecret.Namespace, certSecret.Name), relatedObjects...)
continue
}
}
Expand All @@ -50,38 +50,37 @@ func (t *Translator) getCACerts() []kong.CACertificate {
if err != nil {
relatedObjects := getPluginsAssociatedWithCACertSecret(secretID, t.storer)
relatedObjects = append(relatedObjects, certSecret.DeepCopy())
t.registerTranslationFailure(fmt.Sprintf("invalid CA certificate: %s", err), relatedObjects...)
t.registerTranslationFailure(fmt.Sprintf("invalid secret CA certificate: %s", err), relatedObjects...)
continue
}

caCerts = append(caCerts, caCert)
}

for _, certConfigMap := range caCertConfigMaps {
idBytes, ok := certConfigMap.Data["id"]
certID, ok := certConfigMap.Data["id"]
if !ok {
t.registerTranslationFailure("invalid CA certificate: missing 'id' field in data", certConfigMap)
t.registerTranslationFailure("invalid configmap CA certificate: missing 'id' field in data", certConfigMap)
continue
}
secretID := string(idBytes)

// Allow the certificate key to be named either "cert" or "ca.crt"
caCertbytes, certExists := certConfigMap.Data["cert"]
if !certExists {
caCertbytes, certExists = certConfigMap.Data["ca.crt"]
if !certExists {
relatedObjects := getPluginsAssociatedWithCACertSecret(secretID, t.storer)
relatedObjects := getPluginsAssociatedWithCACertSecret(certID, t.storer)
relatedObjects = append(relatedObjects, certConfigMap.DeepCopy())
t.registerTranslationFailure(fmt.Sprintf("invalid CA certificate: %s", err), relatedObjects...)
t.registerTranslationFailure(fmt.Sprintf(`invalid configmap CA certificate %s/%s, neither "cert" nor "ca.crt" key exist`, certConfigMap.Namespace, certConfigMap.Name), relatedObjects...)
continue
}
}

caCert, err := toKongCACertificate([]byte(caCertbytes), certConfigMap, secretID)
caCert, err := toKongCACertificate([]byte(caCertbytes), certConfigMap, certID)
if err != nil {
relatedObjects := getPluginsAssociatedWithCACertSecret(secretID, t.storer)
relatedObjects := getPluginsAssociatedWithCACertSecret(certID, t.storer)
relatedObjects = append(relatedObjects, certConfigMap.DeepCopy())
t.registerTranslationFailure(fmt.Sprintf("invalid CA certificate: %s", err), relatedObjects...)
t.registerTranslationFailure(fmt.Sprintf("invalid configmap CA certificate: %s", err), relatedObjects...)
continue
}

Expand Down
9 changes: 9 additions & 0 deletions internal/store/fake_store.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ type FakeObjects struct {
Services []*corev1.Service
EndpointSlices []*discoveryv1.EndpointSlice
Secrets []*corev1.Secret
ConfigMaps []*corev1.ConfigMap
KongPlugins []*kongv1.KongPlugin
KongClusterPlugins []*kongv1.KongClusterPlugin
KongIngresses []*kongv1.KongIngress
Expand Down Expand Up @@ -161,6 +162,13 @@ func NewFakeStore(
return nil, err
}
}
configMapStore := cache.NewStore(namespacedKeyFunc)
for _, s := range objects.ConfigMaps {
err := configMapStore.Add(s)
if err != nil {
return nil, err
}
}
endpointSliceStore := cache.NewStore(namespacedKeyFunc)
for _, e := range objects.EndpointSlices {
err := endpointSliceStore.Add(e)
Expand Down Expand Up @@ -248,6 +256,7 @@ func NewFakeStore(
Service: serviceStore,
EndpointSlice: endpointSliceStore,
Secret: secretsStore,
ConfigMap: configMapStore,
Plugin: kongPluginsStore,
ClusterPlugin: kongClusterPluginsStore,
Consumer: consumerStore,
Expand Down
68 changes: 62 additions & 6 deletions internal/store/fake_store_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -531,17 +531,31 @@ func TestFakeStore_ListCACerts(t *testing.T) {
secrets := []*corev1.Secret{
{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Name: "foo-secret",
Namespace: "default",
},
},
}
store, err := NewFakeStore(FakeObjects{Secrets: secrets})
configMaps := []*corev1.ConfigMap{
{
ObjectMeta: metav1.ObjectMeta{
Name: "foo-configmap",
Namespace: "default",
},
},
}
store, err := NewFakeStore(
FakeObjects{
Secrets: secrets,
ConfigMaps: configMaps,
},
)
require.Nil(err)
require.NotNil(store)
certs, _, err := store.ListCACerts()
secretCerts, configMapCerts, err := store.ListCACerts()
assert.Nil(err)
assert.Len(certs, 0)
assert.Len(secretCerts, 0, "expect no secrets as CA certificates")
assert.Len(configMapCerts, 0, "expect no configmaps as CA certificates")

secrets = []*corev1.Secret{
{
Expand Down Expand Up @@ -572,9 +586,51 @@ func TestFakeStore_ListCACerts(t *testing.T) {
store, err = NewFakeStore(FakeObjects{Secrets: secrets})
require.Nil(err)
require.NotNil(store)
certs, _, err = store.ListCACerts()
secretCerts, configMapCerts, err = store.ListCACerts()
assert.Nil(err)
assert.Len(secretCerts, 2, "expect two secrets as CA certificates")
assert.Len(configMapCerts, 0, "expect 0 configmap as CA certificates")

secrets = []*corev1.Secret{
{
ObjectMeta: metav1.ObjectMeta{
Name: "foo-secret",
Namespace: "default",
Labels: map[string]string{
"konghq.com/ca-cert": "true",
},
Annotations: map[string]string{
annotations.IngressClassKey: annotations.DefaultIngressClass,
},
},
},
}
configMaps = []*corev1.ConfigMap{
{
ObjectMeta: metav1.ObjectMeta{
Name: "foo-configmap",
Namespace: "default",
Labels: map[string]string{
"konghq.com/ca-cert": "true",
},
Annotations: map[string]string{
annotations.IngressClassKey: annotations.DefaultIngressClass,
},
},
},
}
store, err = NewFakeStore(
FakeObjects{
Secrets: secrets,
ConfigMaps: configMaps,
},
)
require.Nil(err)
require.NotNil(store)
secretCerts, configMapCerts, err = store.ListCACerts()
assert.Nil(err)
assert.Len(certs, 2, "expect two secrets as CA certificates")
assert.Len(secretCerts, 1, "expect 1 secret as CA certificates")
assert.Len(configMapCerts, 1, "expect 1 configmap as CA certificates")
}

func TestFakeStoreHTTPRoute(t *testing.T) {
Expand Down
6 changes: 3 additions & 3 deletions internal/store/store.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/selection"
"k8s.io/apimachinery/pkg/types"
k8stypes "k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/tools/cache"
"sigs.k8s.io/controller-runtime/pkg/client"
gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"
Expand Down Expand Up @@ -106,7 +106,7 @@ type Storer interface {
ListGRPCRoutes() ([]*gatewayapi.GRPCRoute, error)
ListReferenceGrants() ([]*gatewayapi.ReferenceGrant, error)
ListGateways() ([]*gatewayapi.Gateway, error)
ListBackendTLSPoliciesByTargetService(service types.NamespacedName) ([]*gatewayapi.BackendTLSPolicy, error)
ListBackendTLSPoliciesByTargetService(service k8stypes.NamespacedName) ([]*gatewayapi.BackendTLSPolicy, error)
}

// Store implements Storer and can be used to list Ingress, Services
Expand Down Expand Up @@ -358,7 +358,7 @@ func (s Store) ListGateways() ([]*gatewayapi.Gateway, error) {

// ListBackendTLSPoliciesByTargetService returns the list of BackendTLSPolicies in the BackendTLSPolicy cache store.
// The policies are filtered by the target service.
func (s Store) ListBackendTLSPoliciesByTargetService(service types.NamespacedName) ([]*gatewayapi.BackendTLSPolicy, error) {
func (s Store) ListBackendTLSPoliciesByTargetService(service k8stypes.NamespacedName) ([]*gatewayapi.BackendTLSPolicy, error) {
policiesToReturn := []*gatewayapi.BackendTLSPolicy{}
policies, err := List[*gatewayapi.BackendTLSPolicy](s.stores)
if err != nil {
Expand Down

0 comments on commit c76772f

Please sign in to comment.