Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin/main' into fix-namespacedname-isset
Browse files Browse the repository at this point in the history
  • Loading branch information
czeslavo committed Feb 24, 2023
2 parents e14147e + 7e5b8f9 commit b244958
Show file tree
Hide file tree
Showing 8 changed files with 276 additions and 62 deletions.
10 changes: 3 additions & 7 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -159,16 +159,12 @@ Adding a new version? You'll need three changes:
- Event messages for invalid multi-Service backends now indicate their derived
Kong resource name.
[#3318](https://github.com/Kong/kubernetes-ingress-controller/pull/3318)
- `--konnect-sync-enabled` feature flag has been introduced. It enables the
integration with Kong's Konnect cloud. It's turned off by default.
When enabled, it allows synchronising data-plane configuration with
a Konnect Runtime Group specified by `--konnect-runtime-group-id`.
It requires `--konnect-tls-client-*` set of flags to be set to provide
Runtime Group's TLS client certificates for authentication.
[#3455](https://github.com/Kong/kubernetes-ingress-controller/pull/3455)
- Removed a duplicated status update of the HTTPRoute, which led to a potential
status flickering.
[#3451](https://github.com/Kong/kubernetes-ingress-controller/pull/3451)
- Made Admission Webhook fetch the latest list of Gateways to avoid calling
outdated services set statically during the setup.
[#3601](https://github.com/Kong/kubernetes-ingress-controller/pull/3601)
- Fixed the way configuration flags `KongAdminSvc` and `PublishService` are
checked for being set. The old one was always evaluating to `true`.
[#3602](https://github.com/Kong/kubernetes-ingress-controller/pull/3602)
Expand Down
55 changes: 55 additions & 0 deletions internal/admission/adminapi_provider.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package admission

import (
"github.com/kong/go-kong/kong"

"github.com/kong/kubernetes-ingress-controller/v2/internal/adminapi"
)

// GatewayClientsProvider returns the most recent set of Gateway Admin API clients.
type GatewayClientsProvider interface {
GatewayClients() []*adminapi.Client
}

// DefaultAdminAPIServicesProvider allows getting Admin API services that require having at least one Gateway discovered.
// In the case there's no Gateways, it will return `false` from every method, signalling there's no Gateway available.
type DefaultAdminAPIServicesProvider struct {
gatewayClientsProvider GatewayClientsProvider
}

func NewDefaultAdminAPIServicesProvider(gatewaysProvider GatewayClientsProvider) *DefaultAdminAPIServicesProvider {
return &DefaultAdminAPIServicesProvider{gatewayClientsProvider: gatewaysProvider}
}

func (p DefaultAdminAPIServicesProvider) GetConsumersService() (kong.AbstractConsumerService, bool) {
c, ok := p.designatedAdminAPIClient()
if !ok {
return nil, ok
}
return c.Consumers, true
}

func (p DefaultAdminAPIServicesProvider) GetPluginsService() (kong.AbstractPluginService, bool) {
c, ok := p.designatedAdminAPIClient()
if !ok {
return nil, ok
}
return c.Plugins, true
}

func (p DefaultAdminAPIServicesProvider) designatedAdminAPIClient() (*kong.Client, bool) {
gwClients := p.gatewayClientsProvider.GatewayClients()
if len(gwClients) == 0 {
return nil, false
}

// For now using first client is kind of OK. Using Consumer and Plugin
// services from first kong client should theoretically return the same
// results as for all other clients. There might be instances where
// configurations in different Kong Gateways are ever so slightly
// different but that shouldn't cause a fatal failure.
//
// TODO: We should take a look at this sooner rather than later.
// https://github.com/Kong/kubernetes-ingress-controller/issues/3363
return gwClients[0].AdminAPIClient(), true
}
49 changes: 49 additions & 0 deletions internal/admission/adminapi_provider_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package admission_test

import (
"testing"

"github.com/samber/lo"
"github.com/stretchr/testify/require"

"github.com/kong/kubernetes-ingress-controller/v2/internal/adminapi"
"github.com/kong/kubernetes-ingress-controller/v2/internal/admission"
)

type fakeGatewayClientsProvider struct {
clients []*adminapi.Client
}

func (f fakeGatewayClientsProvider) GatewayClients() []*adminapi.Client {
return f.clients
}

func TestDefaultAdminAPIServicesProvider(t *testing.T) {
t.Run("no clients available should return false from methods", func(t *testing.T) {
p := admission.NewDefaultAdminAPIServicesProvider(fakeGatewayClientsProvider{})

_, ok := p.GetConsumersService()
require.False(t, ok)

_, ok = p.GetPluginsService()
require.False(t, ok)
})

t.Run("when clients available should return first one", func(t *testing.T) {
firstClient := lo.Must(adminapi.NewTestClient("localhost:8080"))
p := admission.NewDefaultAdminAPIServicesProvider(fakeGatewayClientsProvider{
clients: []*adminapi.Client{
firstClient,
lo.Must(adminapi.NewTestClient("localhost:8081")),
},
})

consumersSvc, ok := p.GetConsumersService()
require.True(t, ok)
require.Equal(t, firstClient.AdminAPIClient().Consumers, consumersSvc)

pluginsSvc, ok := p.GetPluginsService()
require.True(t, ok)
require.Equal(t, firstClient.AdminAPIClient().Plugins, pluginsSvc)
})
}
88 changes: 59 additions & 29 deletions internal/admission/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,20 @@ type KongValidator interface {
ValidateHTTPRoute(ctx context.Context, httproute gatewaycontroller.HTTPRoute) (bool, string, error)
}

// AdminAPIServicesProvider provides KongHTTPValidator with Kong Admin API services that are needed to perform
// validation against entities stored by the Gateway.
type AdminAPIServicesProvider interface {
GetConsumersService() (kong.AbstractConsumerService, bool)
GetPluginsService() (kong.AbstractPluginService, bool)
}

// KongHTTPValidator implements KongValidator interface to validate Kong
// entities using the Admin API of Kong.
type KongHTTPValidator struct {
ConsumerSvc kong.AbstractConsumerService
PluginSvc kong.AbstractPluginService
Logger logrus.FieldLogger
SecretGetter kongstate.SecretGetter
ManagerClient client.Client
Logger logrus.FieldLogger
SecretGetter kongstate.SecretGetter
ManagerClient client.Client
AdminAPIServicesProvider AdminAPIServicesProvider

ingressClassMatcher func(*metav1.ObjectMeta, string, annotations.ClassMatching) bool
}
Expand All @@ -47,19 +53,17 @@ type KongHTTPValidator struct {
// such as consumer credentials secrets. If you do not pass a cached client
// here, the performance of this validator can get very poor at high scales.
func NewKongHTTPValidator(
consumerSvc kong.AbstractConsumerService,
pluginSvc kong.AbstractPluginService,
logger logrus.FieldLogger,
managerClient client.Client,
ingressClass string,
servicesProvider AdminAPIServicesProvider,
) KongHTTPValidator {
matcher := annotations.IngressClassValidatorFuncFromObjectMeta(ingressClass)
return KongHTTPValidator{
ConsumerSvc: consumerSvc,
PluginSvc: pluginSvc,
Logger: logger,
SecretGetter: &managerClientSecretGetter{managerClient: managerClient},
ManagerClient: managerClient,
Logger: logger,
SecretGetter: &managerClientSecretGetter{managerClient: managerClient},
ManagerClient: managerClient,
AdminAPIServicesProvider: servicesProvider,

ingressClassMatcher: matcher,
}
Expand All @@ -84,16 +88,9 @@ func (validator KongHTTPValidator) ValidateConsumer(
return false, ErrTextConsumerUsernameEmpty, nil
}

// verify that the consumer is not already otherwise present in the data-plane
c, err := validator.ConsumerSvc.Get(ctx, &consumer.Username)
if err != nil {
if !kong.IsNotFoundErr(err) {
validator.Logger.WithError(err).Error("failed to fetch consumer from kong")
return false, ErrTextConsumerUnretrievable, err
}
}
if c != nil {
return false, ErrTextConsumerExists, nil
errText, err := validator.ensureConsumerDoesNotExistInGateway(ctx, consumer.Username)
if err != nil || errText != "" {
return false, errText, err
}

// if there are no credentials for this consumer, there's no need to move on
Expand Down Expand Up @@ -254,14 +251,12 @@ func (validator KongHTTPValidator) ValidatePlugin(
if len(k8sPlugin.Protocols) > 0 {
plugin.Protocols = kong.StringSlice(kongv1.KongProtocolsToStrings(k8sPlugin.Protocols)...)
}
isValid, msg, err := validator.PluginSvc.Validate(ctx, &plugin)
if err != nil {
return false, ErrTextPluginConfigValidationFailed, err
}
if !isValid {
return isValid, fmt.Sprintf(ErrTextPluginConfigViolatesSchema, msg), nil
errText, err := validator.validatePluginAgainstGatewaySchema(ctx, plugin)
if err != nil || errText != "" {
return false, errText, err
}
return isValid, "", nil

return true, "", nil
}

// ValidateClusterPlugin transfers relevant fields from a KongClusterPlugin into a KongPlugin and then returns
Expand Down Expand Up @@ -395,6 +390,41 @@ func (validator KongHTTPValidator) listManagedConsumers(ctx context.Context) ([]
return managedConsumers, nil
}

func (validator KongHTTPValidator) ensureConsumerDoesNotExistInGateway(ctx context.Context, username string) (string, error) {
if consumerSvc, hasClient := validator.AdminAPIServicesProvider.GetConsumersService(); hasClient {
// verify that the consumer is not already present in the data-plane
c, err := consumerSvc.Get(ctx, &username)
if err != nil {
if !kong.IsNotFoundErr(err) {
validator.Logger.WithError(err).Error("failed to fetch consumer from kong")
return ErrTextConsumerUnretrievable, err
}
}
if c != nil {
return ErrTextConsumerExists, nil
}
}

// if there's no client, do not verify existence with data-plane as there's none available
return "", nil
}

func (validator KongHTTPValidator) validatePluginAgainstGatewaySchema(ctx context.Context, plugin kong.Plugin) (string, error) {
pluginService, hasClient := validator.AdminAPIServicesProvider.GetPluginsService()
if hasClient {
isValid, msg, err := pluginService.Validate(ctx, &plugin)
if err != nil {
return ErrTextPluginConfigValidationFailed, err
}
if !isValid {
return fmt.Sprintf(ErrTextPluginConfigViolatesSchema, msg), nil
}
}

// if there's no client, do not verify with data-plane as there's none available
return "", nil
}

// -----------------------------------------------------------------------------
// Private - Manager Client Secret Getter
// -----------------------------------------------------------------------------
Expand Down
Loading

0 comments on commit b244958

Please sign in to comment.