Skip to content

Commit

Permalink
do not allow setting proxy and route
Browse files Browse the repository at this point in the history
  • Loading branch information
czeslavo committed Oct 31, 2023
1 parent f191145 commit 35c62b6
Show file tree
Hide file tree
Showing 14 changed files with 97 additions and 24 deletions.
9 changes: 5 additions & 4 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -130,10 +130,11 @@ Adding a new version? You'll need three changes:
expected to functionally affect routing, but may affect performance for
some configurations.
[#4934](https://github.com/Kong/kubernetes-ingress-controller/pull/4934)
- KongIngress is deprecated and will be removed in a future release. Settings
previously managed by KongIngress must be migrated to annotations and the new
KongUpstreamPolicy resource.
[#4989](https://github.com/Kong/kubernetes-ingress-controller/pull/4989)
- KongIngress is now entirely deprecated and will be removed in a future release.
Its fields that were previously deprecated (`proxy` and `route`) are now not allowed to be set.
They must be migrated to annotations. `upstream` field is deprecated - it's recommended
to migrate its settings to the new KongUpstreamPolicy resource.
[#5029](https://github.com/Kong/kubernetes-ingress-controller/pull/5029)

### Fixed

Expand Down
7 changes: 7 additions & 0 deletions config/crd/bases/configuration.konghq.com_kongingresses.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,13 @@ spec:
type: integer
type: object
type: object
x-kubernetes-validations:
- message: '''proxy'' field is no longer supported, use Service''s annotations
instead'
rule: '!has(self.proxy)'
- message: '''route'' field is no longer supported, use Ingress'' annotations
instead'
rule: '!has(self.route)'
served: true
storage: true
subresources:
Expand Down
5 changes: 3 additions & 2 deletions internal/admission/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -313,10 +313,11 @@ func (h RequestHandler) handleKongIngress(_ context.Context, request admissionv1
// KongIngress is always allowed.
responseBuilder = responseBuilder.Allowed(true)

// Proxy and Route fields are now disallowed to be set with the use of CEL rules in the CRD definition.
// We still warn about them here only just in case someone doesn't install new CRDs with CEL rules.
if kongIngress.Proxy != nil {
responseBuilder = responseBuilder.WithWarning(proxyWarning)
}

if kongIngress.Route != nil {
responseBuilder = responseBuilder.WithWarning(routeWarning)
}
Expand Down Expand Up @@ -345,7 +346,7 @@ func (h RequestHandler) handleService(_ context.Context, request admissionv1.Adm

if annotations.ExtractConfigurationName(service.Annotations) != "" {
warning := fmt.Sprintf(serviceWarning, annotations.AnnotationPrefix+annotations.ConfigurationKey,
annotations.AnnotationPrefix+kongv1beta1.KongUpstreamPolicyAnnotationKey)
kongv1beta1.KongUpstreamPolicyAnnotationKey)

responseBuilder = responseBuilder.WithWarning(warning)
}
Expand Down
2 changes: 1 addition & 1 deletion internal/admission/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ func TestHandleService(t *testing.T) {
},
wantWarnings: []string{
fmt.Sprintf(serviceWarning, annotations.AnnotationPrefix+annotations.ConfigurationKey,
annotations.AnnotationPrefix+kongv1beta1.KongUpstreamPolicyAnnotationKey),
kongv1beta1.KongUpstreamPolicyAnnotationKey),
},
},
{
Expand Down
8 changes: 4 additions & 4 deletions internal/dataplane/kongstate/kongstate.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ func (ks *KongState) FillUpstreamOverrides(
logger.Error(nil, fmt.Sprintf(
"Service uses deprecated %s annotation and KongIngress, migrate to %s and KongUpstreamPolicy",
annotations.AnnotationPrefix+annotations.ConfigurationKey,
annotations.AnnotationPrefix+kongv1beta1.KongUpstreamPolicyAnnotationKey),
kongv1beta1.KongUpstreamPolicyAnnotationKey),
"namespace", svc.Namespace, "name", svc.Name)
}
}
Expand All @@ -241,9 +241,9 @@ func (ks *KongState) FillUpstreamOverrides(
fmt.Sprintf("Service uses both %s and %s annotations, should use only %s annotation. Settings "+
"from %s will take precedence",
annotations.AnnotationPrefix+annotations.ConfigurationKey,
annotations.AnnotationPrefix+kongv1beta1.KongUpstreamPolicyAnnotationKey,
annotations.AnnotationPrefix+kongv1beta1.KongUpstreamPolicyAnnotationKey,
annotations.AnnotationPrefix+kongv1beta1.KongUpstreamPolicyAnnotationKey),
kongv1beta1.KongUpstreamPolicyAnnotationKey,
kongv1beta1.KongUpstreamPolicyAnnotationKey,
kongv1beta1.KongUpstreamPolicyAnnotationKey),
"namespace", svc.Namespace, "name", svc.Name)
}
}
Expand Down
5 changes: 2 additions & 3 deletions pkg/apis/configuration/v1/kongingress_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ import (
// +kubebuilder:storageversion
// +kubebuilder:resource:shortName=ki,categories=kong-ingress-controller
// +kubebuilder:validation:Optional
// +kubebuilder:validation:XValidation:rule="!has(self.proxy)", message="'proxy' field is no longer supported, use Service's annotations instead"
// +kubebuilder:validation:XValidation:rule="!has(self.route)", message="'route' field is no longer supported, use Ingress' annotations instead"

// KongIngress is the Schema for the kongingresses API.
type KongIngress struct {
Expand Down Expand Up @@ -210,9 +212,6 @@ type KongIngressUpstream struct {

// HashFallbackURICapture is the "hash_fallback" version of HashOnURICapture.
HashFallbackURICapture *string `json:"hash_fallback_uri_capture,omitempty" yaml:"hash_fallback_uri_capture,omitempty"`

// we need to check this one TODO https://github.com/Kong/kubernetes-ingress-controller/issues/2075
// ClientCertificate *CertificateSecretRef `json:"client_certificate,omitempty" yaml:"client_certificate,omitempty"`
}

func init() {
Expand Down
7 changes: 7 additions & 0 deletions test/e2e/manifests/all-in-one-dbless-k4k8s-enterprise.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions test/e2e/manifests/all-in-one-dbless-konnect-enterprise.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions test/e2e/manifests/all-in-one-dbless-konnect.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions test/e2e/manifests/all-in-one-dbless.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions test/e2e/manifests/all-in-one-postgres-enterprise.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions test/e2e/manifests/all-in-one-postgres.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

29 changes: 29 additions & 0 deletions test/envtest/crds_envtest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"github.com/go-logr/zapr"
"github.com/google/uuid"
kongv1 "github.com/kong/kubernetes-ingress-controller/v3/pkg/apis/configuration/v1"
"github.com/samber/lo"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -457,6 +458,28 @@ func TestCRDValidations(t *testing.T) {
require.ErrorContains(t, err, `spec.hashOnFallback must not be set when spec.hashOn.cookie is set.`)
},
},
{
name: "KongIngress - proxy is not allowed",
scenario: func(ctx context.Context, t *testing.T, ns string) {
err := createKongIngress(ctx, ctrlClient, ns, &kongv1.KongIngress{
Proxy: &kongv1.KongIngressService{
Retries: lo.ToPtr(5),
},
})
require.ErrorContains(t, err, "'proxy' field is no longer supported, use Service's annotations instead")
},
},
{
name: "KongIngress - route is not allowed",
scenario: func(ctx context.Context, t *testing.T, ns string) {
err := createKongIngress(ctx, ctrlClient, ns, &kongv1.KongIngress{
Route: &kongv1.KongIngressRoute{
PreserveHost: lo.ToPtr(true),
},
})
require.ErrorContains(t, err, "'route' field is no longer supported, use Ingress' annotations instead")
},
},
}

for _, tc := range testCases {
Expand Down Expand Up @@ -598,3 +621,9 @@ func generateInvalidHashOns() []kongv1beta1.KongUpstreamHash {
return fmt.Sprintf("%s.%s.%s.%s", optStr(h.Cookie), optStr(h.Header), optStr(h.URICapture), optStr(h.QueryArg))
})
}

func createKongIngress(ctx context.Context, client client.Client, ns string, ingress *kongv1.KongIngress) error {
ingress.GenerateName = "test-"
ingress.Namespace = ns
return client.Create(ctx, ingress)
}
14 changes: 4 additions & 10 deletions test/integration/kongingress_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,9 @@ func TestKongIngressValidationWebhook(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
GenerateName: "kong-ingress-validation-",
},
// Proxy field is deprecated, expecting warning for it.
Proxy: &kongv1.KongIngressService{
Protocol: lo.ToPtr("tcp"),
},
// Route field is deprecated, expecting warning for it.
Route: &kongv1.KongIngressRoute{
Methods: []*string{lo.ToPtr("POST")},
// Upstream field is deprecated, expecting warning for it.
Upstream: &kongv1.KongIngressUpstream{
HashOn: lo.ToPtr("none"),
},
}

Expand All @@ -73,10 +69,8 @@ func TestKongIngressValidationWebhook(t *testing.T) {
Do(ctx)

assert.NoError(t, result.Error())
require.Len(t, result.Warnings(), 2)
expectedWarnings := []string{
"Support for 'proxy' was removed in 3.0. It will have no effect. Use Service's annotations instead.",
"Support for 'route' was removed in 3.0. It will have no effect. Use Ingress' annotations instead.",
"'upstream' is DEPRECATED. It will have no effect. Use KongUpstreamPolicy instead.",
}
receivedWarnings := lo.Map(result.Warnings(), func(item net.WarningHeader, index int) string {
return item.Text
Expand Down

0 comments on commit 35c62b6

Please sign in to comment.