Skip to content

Commit

Permalink
v2alpha1 support for short hosts (#1311)
Browse files Browse the repository at this point in the history 
* Short host validation

* More validation

* Default domain from non-kyma gateway

* State

* State

* integration test

* Renamings and docs update

* Update internal/validation/v2alpha1/hosts.go

Co-authored-by: Bartosz Chwila <[email protected]>

* Review naming suggestions

* Update error message

* Cover case for Istio gateway with host definition "*."

* Fix

* Error on APIRule when Gateway not found

* Fix integration test

* Update internal/validation/v2alpha1/hosts.go

Co-authored-by: Bartosz Chwila <[email protected]>

* Update docs/contributor/adr/0007-apirule-v2alpha1-api-proposal.md

Co-authored-by: Bartosz Chwila <[email protected]>

* Update internal/helpers/hosts.go

Co-authored-by: Bartosz Chwila <[email protected]>

* Review adjustments

* Fix lint

* Review suggestion re-correction

* Fix int test v2alpha1 status checks and adds error int scenario for short host name

* Unwanted removal

* Fix error message

* Revert v2alpha1 integration test use new status check func

* Update docs/user/custom-resources/apirule/v2alpha1/04-10-apirule-custom-resource.md

Co-authored-by: Natalia Sitko <[email protected]>

* Update docs/user/custom-resources/apirule/v2alpha1/04-10-apirule-custom-resource.md

Co-authored-by: Natalia Sitko <[email protected]>

* Fix v2alpha1 int tests with original function for applying

---------

Co-authored-by: Bartosz Chwila <[email protected]>
Co-authored-by: Natalia Sitko <[email protected]>
  • Loading branch information
@videlov @barchw @nataliasitko
3 people authored Sep 26, 2024
1 parent 0f086ab commit e4d8e37
Show file tree
Hide file tree
Showing 48 changed files with 1,663 additions and 641 deletions.
5 changes: 2 additions & 3 deletions apis/gateway/v2alpha1/apirule_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,9 @@ type APIRuleSpec struct {
Timeout *Timeout `json:"timeout,omitempty"`
}

// Host is the URL of the exposed service.
// +kubebuilder:validation:MinLength=3
// Host is the URL of the exposed service. We support lowercase RFC 1123 labels and FQDN.
// +kubebuilder:validation:MaxLength=255
// +kubebuilder:validation:XValidation:rule=`self.matches('^(?:[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?\\.)+[a-z0-9][a-z0-9-]{0,61}[a-z0-9]$')`,message="Host is not Fully Qualified Domain Name"
// +kubebuilder:validation:XValidation:rule=`self.matches('^(?:[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?)(?:\\.[a-z0-9][a-z0-9-]{0,61}[a-z0-9])*$')`,message="Host must be a lowercase RFC 1123 label (must consist of lowercase alphanumeric characters or '-', and must start and end with an lowercase alphanumeric character) or a fully qualified domain name"
type Host string

// APIRuleStatus describes the observed state of ApiRule.
Expand Down
11 changes: 7 additions & 4 deletions config/crd/bases/gateway.kyma-project.io_apirules.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -363,13 +363,16 @@ spec:
hosts:
description: Specifies the URLs of the exposed service.
items:
description: Host is the URL of the exposed service.
description: Host is the URL of the exposed service. We support
lowercase RFC 1123 labels and FQDN.
maxLength: 255
minLength: 3
type: string
x-kubernetes-validations:
- message: Host is not Fully Qualified Domain Name
rule: self.matches('^(?:[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?\\.)+[a-z0-9][a-z0-9-]{0,61}[a-z0-9]$')
- message: Host must be a lowercase RFC 1123 label (must consist
of lowercase alphanumeric characters or '-', and must start
and end with an lowercase alphanumeric character) or a fully
qualified domain name
rule: self.matches('^(?:[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?)(?:\\.[a-z0-9][a-z0-9-]{0,61}[a-z0-9])*$')
maxItems: 1
minItems: 1
type: array
Expand Down
899 changes: 465 additions & 434 deletions controllers/gateway/api_controller_integration_test.go
View file

Large diffs are not rendered by default.

52 changes: 40 additions & 12 deletions controllers/gateway/apirule_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,14 @@ package gateway
import (
"context"
"fmt"
"strings"
"time"

"github.com/kyma-project/api-gateway/internal/dependencies"
"github.com/kyma-project/api-gateway/internal/processing/processors/migration"
"github.com/kyma-project/api-gateway/internal/processing/status"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"time"

gatewayv1beta1 "github.com/kyma-project/api-gateway/apis/gateway/v1beta1"
gatewayv2alpha1 "github.com/kyma-project/api-gateway/apis/gateway/v2alpha1"
Expand All @@ -44,6 +47,7 @@ import (

"github.com/go-logr/logr"
"github.com/kyma-project/api-gateway/internal/processing"
networkingv1beta1 "istio.io/client-go/pkg/apis/networking/v1beta1"
corev1 "k8s.io/api/core/v1"
apierrs "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/types"
Expand Down Expand Up @@ -77,7 +81,7 @@ func (r *APIRuleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
l.Info("Starting reconciliation")
ctx = logr.NewContext(ctx, r.Log)

defaultDomainName, err := default_domain.GetDefaultDomainFromKymaGateway(ctx, r.Client)
defaultDomainName, err := default_domain.GetDomainFromKymaGateway(ctx, r.Client)
if err != nil && default_domain.HandleDefaultDomainError(l, err) {
return doneReconcileErrorRequeue(err, errorReconciliationPeriod)
}
Expand Down Expand Up @@ -117,11 +121,11 @@ func (r *APIRuleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
}

if r.isApiRuleConvertedFromV2alpha1(apiRule) {
return r.reconcileV2alpha1APIRule(ctx, l, apiRule, defaultDomainName)
return r.reconcileV2Alpha1APIRule(ctx, l, apiRule)
}

l.Info("Reconciling v1beta1 APIRule", "jwtHandler", r.Config.JWTHandler)
cmd := r.getV1beta1Reconciliation(&apiRule, defaultDomainName, &l)
cmd := r.getV1Beta1Reconciliation(&apiRule, defaultDomainName, &l)
if name, err := dependencies.APIRule().AreAvailable(ctx, r.Client); err != nil {
s, err := handleDependenciesError(name, err).V1beta1Status()
if err != nil {
Expand Down Expand Up @@ -157,7 +161,7 @@ func (r *APIRuleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
return r.updateStatus(ctx, l, &apiRule)
}

func (r *APIRuleReconciler) reconcileV2alpha1APIRule(ctx context.Context, l logr.Logger, apiRule gatewayv1beta1.APIRule, domain string) (ctrl.Result, error) {
func (r *APIRuleReconciler) reconcileV2Alpha1APIRule(ctx context.Context, l logr.Logger, apiRule gatewayv1beta1.APIRule) (ctrl.Result, error) {
l.Info("Reconciling v2alpha1 APIRule")
toUpdate := apiRule.DeepCopy()
migrate, err := apiRuleNeedsMigration(ctx, r.Client, toUpdate)
Expand All @@ -179,8 +183,33 @@ func (r *APIRuleReconciler) reconcileV2alpha1APIRule(ctx context.Context, l logr
if err := rule.ConvertFrom(toUpdate); err != nil {
return doneReconcileErrorRequeue(err, r.OnErrorReconcilePeriod)
}
cmd := r.getv2alpha1Reconciliation(&apiRule, &rule,
domain, migrate, &l)

gatewayName := strings.Split(*rule.Spec.Gateway, "/")
gatewayNN := types.NamespacedName{
Namespace: gatewayName[0],
Name: gatewayName[1],
}
var gateway networkingv1beta1.Gateway
if err := r.Client.Get(ctx, gatewayNN, &gateway); err != nil {
v2Alpha1Status := status.ReconciliationV2alpha1Status{
ApiRuleStatus: &gatewayv2alpha1.APIRuleStatus{
State: gatewayv2alpha1.State(gatewayv2alpha1.Error),
},
}
s := v2Alpha1Status.GenerateStatusFromFailures([]validation.Failure{
{
AttributePath: "spec.gateway",
Message: "Could not get specified Gateway",
},
})
if err := s.UpdateStatus(&rule.Status); err != nil {
l.Error(err, "Error updating APIRule status")
return doneReconcileErrorRequeue(err, r.OnErrorReconcilePeriod)
}
return r.convertAndUpdateStatus(ctx, l, rule)
}

cmd := r.getV2Alpha1Reconciliation(&apiRule, &rule, &gateway, migrate, &l)

if name, err := dependencies.APIRule().AreAvailable(ctx, r.Client); err != nil {
s, err := handleDependenciesError(name, err).V2alpha1Status()
Expand Down Expand Up @@ -256,9 +285,9 @@ func handleDependenciesError(name string, err error) controllers.Status {
}
}

func (r *APIRuleReconciler) getV1beta1Reconciliation(apiRule *gatewayv1beta1.APIRule, defaultDomain string, namespacedLogger *logr.Logger) processing.ReconciliationCommand {
func (r *APIRuleReconciler) getV1Beta1Reconciliation(apiRule *gatewayv1beta1.APIRule, defaultDomainName string, namespacedLogger *logr.Logger) processing.ReconciliationCommand {
config := r.ReconciliationConfig
config.DefaultDomainName = defaultDomain
config.DefaultDomainName = defaultDomainName
switch {
case r.Config.JWTHandler == helpers.JWT_HANDLER_ISTIO:
return istio.NewIstioReconciliation(apiRule, config, namespacedLogger)
Expand All @@ -267,11 +296,10 @@ func (r *APIRuleReconciler) getV1beta1Reconciliation(apiRule *gatewayv1beta1.API
}
}

func (r *APIRuleReconciler) getv2alpha1Reconciliation(apiRulev1beta1 *gatewayv1beta1.APIRule, apiRulev2alpha1 *gatewayv2alpha1.APIRule, defaultDomain string, needsMigration bool, namespacedLogger *logr.Logger) processing.ReconciliationCommand {
func (r *APIRuleReconciler) getV2Alpha1Reconciliation(apiRulev1beta1 *gatewayv1beta1.APIRule, apiRulev2alpha1 *gatewayv2alpha1.APIRule, gateway *networkingv1beta1.Gateway, needsMigration bool, namespacedLogger *logr.Logger) processing.ReconciliationCommand {
config := r.ReconciliationConfig
config.DefaultDomainName = defaultDomain
v2alpha1Validator := v2alpha1.NewAPIRuleValidator(apiRulev2alpha1)
return v2alpha1Processing.NewReconciliation(apiRulev2alpha1, apiRulev1beta1, v2alpha1Validator, config, namespacedLogger, needsMigration)
return v2alpha1Processing.NewReconciliation(apiRulev2alpha1, apiRulev1beta1, gateway, v2alpha1Validator, config, namespacedLogger, needsMigration)
}

// SetupWithManager sets up the controller with the Manager.
Expand Down
2 changes: 1 addition & 1 deletion docs/contributor/adr/0007-apirule-v2alpha1-api-proposal.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ Due to the deprecation of Ory and the introduction of new features in API Gatewa
| **corsPolicy.allowCredentials** | **NO** | Specifies whether credentials are allowed in the **Access-Control-Allow-Credentials** CORS header. | |
| **corsPolicy.exposeHeaders** | **NO** | Specifies headers exposed with the **Access-Control-Expose-Headers** CORS header. | |
| **corsPolicy.maxAge** | **NO** | Specifies the maximum age of CORS policy cache. The value is provided in the **Access-Control-Max-Age** CORS header. | |
| **hosts** | **YES** | Specifies the Service's communication address for inbound external traffic. If only the leftmost label is provided, the default domain name is used. | The full domain name or the leftmost label cannot contain the wildcard character `*`. |
| **hosts** | **YES** | Specifies the Service's communication address for inbound external traffic. If only the leftmost label is provided, the domain name from the referenced Gateway is used, expanding the host to `<label>.<gateway domain>`. | The full domain name or the leftmost label cannot contain the wildcard character `*`. |
| **service.name** | **NO** | Specifies the name of the exposed Service. | |
| **service.namespace** | **NO** | Specifies the namespace of the exposed Service. | |
| **service.port** | **NO** | Specifies the communication port of the exposed Service. | |
Expand Down
5 changes: 4 additions & 1 deletion docs/release-notes/2.7.0.md
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
## New Features

- Add support for short host name for APIRule in version `v2alpha1` [#1311](https://github.com/kyma-project/api-gateway/pull/1311)

## Bug fixes

- Fix the wildcard path format and the validation of the rule path in an APIRule [#1285](https://github.com/kyma-project/api-gateway/pull/1285)
- Fix the wildcard path format and the validation of the rule path in an APIRule [#1285](https://github.com/kyma-project/api-gateway/pull/1285)
27 changes: 26 additions & 1 deletion docs/user/custom-resources/apirule/v2alpha1/04-10-apirule-custom-resource.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ This table lists all parameters of APIRule `v2alpha1` CRD together with their de
| **corsPolicy.allowCredentials** | **NO** | Specifies whether credentials are allowed in the **Access-Control-Allow-Credentials** CORS header. | None |
| **corsPolicy.exposeHeaders** | **NO** | Specifies headers exposed with the **Access-Control-Expose-Headers** CORS header. | None |
| **corsPolicy.maxAge** | **NO** | Specifies the maximum age of CORS policy cache. The value is provided in the **Access-Control-Max-Age** CORS header. | None |
| **hosts** | **YES** | Specifies the Service's communication address for inbound external traffic. It must be a fully qualified domain name in proper FQDN format: at least two domain labels with characters, numbers, or dashes. | FQDN format. |
| **hosts** | **YES** | Specifies the Service's communication address for inbound external traffic. It must be a RFC 1123 label or a valid, fully qualified domain name (FQDN) in the following format: at least two domain labels with characters, numbers, or hyphens. | Lowercase RFC 1123 label or FQDN format. |
| **service.name** | **NO** | Specifies the name of the exposed Service. | None |
| **service.namespace** | **NO** | Specifies the namespace of the exposed Service. | None |
| **service.port** | **NO** | Specifies the communication port of the exposed Service. | None |
Expand Down Expand Up @@ -61,6 +61,9 @@ This table lists all parameters of APIRule `v2alpha1` CRD together with their de
> [!WARNING]
> If a service is not defined at the **spec.service** level, all defined Access Rules must have it defined at the **spec.rules.service** level. Otherwise, the validation fails.
> [!WARNING]
> If a short host name is defined at the **spec.hosts** level, the referenced Gateway must provide the same single host for all [Server](https://istio.io/latest/docs/reference/config/networking/gateway/#Server) definitions and it must be prefixed with `*.`. Otherwise, the validation fails.
**Status:**

The following table lists the fields of the **status** section.
Expand Down Expand Up @@ -93,3 +96,25 @@ spec:
methods: [ "GET" ]
noAuth: true
```
This sample APIRule illustrates the usage of a short host name. It uses the domain from the referenced Gateway `kyma-system/kyma-gateway`:

```yaml
apiVersion: gateway.kyma-project.io/v2alpha1
kind: APIRule
metadata:
name: service-exposed
spec:
gateway: kyma-system/kyma-gateway
hosts:
- foo
service:
name: foo-service
namespace: foo-namespace
port: 8080
timeout: 360
rules:
- path: /*
methods: [ "GET" ]
noAuth: true
```
Loading

0 comments on commit e4d8e37

Please sign in to comment.