Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: get Admin API services dynamically in admission webhook #3601

Merged
merged 1 commit into from
Feb 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)

### Under the hood

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