Skip to content

Commit

Permalink
fix: validate that started APIs have plans
Browse files Browse the repository at this point in the history
  • Loading branch information
a-cordier authored and kamiiiel committed Sep 11, 2024
1 parent 1b5ee60 commit e91186b
Show file tree
Hide file tree
Showing 12 changed files with 154 additions and 55 deletions.
4 changes: 4 additions & 0 deletions api/model/api/base/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,3 +106,7 @@ type ResponseTemplate struct {
Headers map[string]string `json:"headers,omitempty"`
Body string `json:"body,omitempty"`
}

func (api *ApiBase) GetName() string {
return api.Name
}
8 changes: 8 additions & 0 deletions api/model/api/v2/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,14 @@ func (api *Api) GetDefinitionVersion() core.ApiDefinitionVersion {
return core.ApiV2
}

func (api *Api) GetState() string {
return string(api.State)
}

func (api *Api) HasPlans() bool {
return len(api.Plans) > 0
}

func (api *Api) GetContextPaths() []string {
paths := make([]string, 0)
proxy := api.Proxy
Expand Down
8 changes: 8 additions & 0 deletions api/model/api/v4/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,14 @@ func (api *Api) SetDefinitionContext(ctx core.DefinitionContext) {
}
}

func (api *Api) GetState() string {
return string(api.State)
}

func (api *Api) HasPlans() bool {
return len(api.Plans) > 0
}

// Converts the API to its gateway definition equivalent.
func (api *Api) ToGatewayDefinition() GatewayDefinitionApi {
def := GatewayDefinitionApi{Api: api}
Expand Down
8 changes: 8 additions & 0 deletions api/v1alpha1/apiv2definition_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,14 @@ func (api *ApiDefinition) SetDefinitionContext(ctx core.DefinitionContext) {
api.Spec.SetDefinitionContext(ctx)
}

func (api *ApiDefinition) GetState() string {
return api.Spec.GetState()
}

func (api *ApiDefinition) HasPlans() bool {
return api.Spec.HasPlans()
}

func (api *ApiDefinition) GetRef() core.ObjectRef {
return &refs.NamespacedName{
Name: api.Name,
Expand Down
8 changes: 8 additions & 0 deletions api/v1alpha1/apiv4definition_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,14 @@ func (api *ApiV4Definition) GetResources() []core.ObjectOrRef[core.ResourceModel
return api.Spec.GetResources()
}

func (api *ApiV4Definition) GetState() string {
return api.Spec.GetState()
}

func (api *ApiV4Definition) HasPlans() bool {
return api.Spec.HasPlans()
}

func (api *ApiV4Definition) PopulateIDs(context core.ContextModel) {
api.Spec.ID = api.pickID(context)
api.Spec.CrossID = api.pickCrossID()
Expand Down
32 changes: 32 additions & 0 deletions internal/admission/api/base/plans.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// Copyright (C) 2015 The Gravitee team (http://gravitee.io)
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package base

import (
"github.com/gravitee-io/gravitee-kubernetes-operator/internal/core"
"github.com/gravitee-io/gravitee-kubernetes-operator/internal/errors"
)

func validatePlans(api core.ApiDefinitionObject) *errors.AdmissionError {
if !api.HasPlans() && api.GetState() != "STOPPED" {
return errors.NewSeveref(
"cannot apply API [%s]. "+
"Its state is set to STARTED, but the API has no plans. "+
"APIs must have at least one plan in order to be deployed.",
api.GetName(),
)
}
return nil
}
1 change: 1 addition & 0 deletions internal/admission/api/base/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ func ValidateCreate(ctx context.Context, obj runtime.Object) *errors.AdmissionEr
errs.Add(ctxref.Validate(ctx, obj))

if api, ok := obj.(core.ApiDefinitionObject); ok {
errs.Add(validatePlans(api))
errs.Add(validateNoConflictingPath(ctx, api))
errs.MergeWith(validateResourceOrRefs(ctx, api))
}
Expand Down
23 changes: 0 additions & 23 deletions internal/admission/api/v4/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ package v4
import (
"context"

baseModel "github.com/gravitee-io/gravitee-kubernetes-operator/api/model/api/base"

v4 "github.com/gravitee-io/gravitee-kubernetes-operator/api/model/api/v4"
"github.com/gravitee-io/gravitee-kubernetes-operator/internal/admission/api/base"
"github.com/gravitee-io/gravitee-kubernetes-operator/internal/apim"
Expand All @@ -35,7 +33,6 @@ func validateCreate(ctx context.Context, obj runtime.Object) *errors.AdmissionEr
return errs
}

errs.Add(validateApiPlans(ctx, api))
if errs.IsSevere() {
return errs
}
Expand All @@ -50,26 +47,6 @@ func validateCreate(ctx context.Context, obj runtime.Object) *errors.AdmissionEr
return errs
}

func validateApiPlans(_ context.Context, api core.ApiDefinitionObject) *errors.AdmissionError {
cp, _ := api.DeepCopyObject().(core.ApiDefinitionObject)

apiDef, ok := cp.GetDefinition().(*v4.Api)
if !ok {
return errors.NewSevere("unable to validate the CRD because it is not a v4 API")
}

if apiDef.State == baseModel.StateStarted &&
len(apiDef.Plans) == 0 {
return errors.NewSeveref(
"cannot apply API [%s]. Its state is set to STARTED but the API has no plans. "+
"APIs must have at least one plan in order to be deployed.",
apiDef.Name,
)
}

return nil
}

func validateDryRun(ctx context.Context, api core.ApiDefinitionObject) *errors.AdmissionErrors {
errs := errors.NewAdmissionErrors()

Expand Down
3 changes: 3 additions & 0 deletions internal/core/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,14 @@ type DefinitionContext interface {

// +k8s:deepcopy-gen=false
type ApiDefinitionModel interface {
GetName() string
GetDefinitionVersion() ApiDefinitionVersion
GetContextPaths() []string
SetDefinitionContext(DefinitionContext)
GetDefinitionContext() DefinitionContext
GetResources() []ObjectOrRef[ResourceModel]
GetState() string
HasPlans() bool
}

// +k8s:deepcopy-gen=false
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// Copyright (C) 2015 The Gravitee team (http://gravitee.io)
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package v2

import (
"context"

"github.com/gravitee-io/gravitee-kubernetes-operator/api/model/api/base"
adm "github.com/gravitee-io/gravitee-kubernetes-operator/internal/admission/api/v2"
"github.com/gravitee-io/gravitee-kubernetes-operator/internal/errors"
"github.com/gravitee-io/gravitee-kubernetes-operator/test/internal/integration/constants"
"github.com/gravitee-io/gravitee-kubernetes-operator/test/internal/integration/fixture"
"github.com/gravitee-io/gravitee-kubernetes-operator/test/internal/integration/labels"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)

var _ = Describe("Validate create", labels.WithoutContext, func() {
ctx := context.Background()
admissionCtrl := adm.AdmissionCtrl{}

It("should return error on API creation with no plans and state 'STARTED'", func() {
fixtures := fixture.
Builder().
WithAPI(constants.Api).
Build()

By("removing plans while API is started")

fixtures.API.Spec.Plans = nil
fixtures.API.Spec.State = base.StateStarted

By("checking that API creation does not pass validation")

_, err := admissionCtrl.ValidateCreate(ctx, fixtures.API)

Expect(err).To(Equal(
errors.NewSeveref(
"cannot apply API [%s]. "+
"Its state is set to STARTED, but the API has no plans. "+
"APIs must have at least one plan in order to be deployed.",
fixtures.API.GetName(),
),
))
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (

v4 "github.com/gravitee-io/gravitee-kubernetes-operator/internal/admission/api/v4"
"github.com/gravitee-io/gravitee-kubernetes-operator/internal/errors"
"github.com/gravitee-io/gravitee-kubernetes-operator/test/internal/integration/assert"
"github.com/gravitee-io/gravitee-kubernetes-operator/test/internal/integration/constants"
"github.com/gravitee-io/gravitee-kubernetes-operator/test/internal/integration/fixture"
"github.com/gravitee-io/gravitee-kubernetes-operator/test/internal/integration/labels"
Expand All @@ -28,31 +27,29 @@ import (
)

var _ = Describe("Validate create", labels.WithContext, func() {
interval := constants.Interval
ctx := context.Background()
admissionCtrl := v4.AdmissionCtrl{}

It("should return error on API creation with no plans and state STARTED", func() {
fixtures := fixture.
Builder().
WithAPIv4(constants.ApiV4).
Build().
Apply()
Build()

By("removing existing plans")

clear(fixtures.APIv4.Spec.Plans)

By("checking that API validation returns errors")
Eventually(func() error {
_, err := admissionCtrl.ValidateCreate(ctx, fixtures.APIv4)

return assert.Equals(
"severe",
errors.NewSeveref("cannot apply API [%s]. Its state is set to STARTED"+
" but the API has no plans. APIs must have at least one plan in order to"+
" be deployed.", fixtures.APIv4.Name).Error(),
err.Error(),
)
}, constants.EventualTimeout, interval).Should(Succeed())

_, err := admissionCtrl.ValidateCreate(ctx, fixtures.APIv4)

Expect(err).To(Equal(
errors.NewSeveref("cannot apply API [%s]. Its state is set to STARTED, "+
"but the API has no plans. APIs must have at least one plan in order to "+
"be deployed.",
fixtures.APIv4.Name,
),
))
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (

v4 "github.com/gravitee-io/gravitee-kubernetes-operator/internal/admission/api/v4"
"github.com/gravitee-io/gravitee-kubernetes-operator/internal/errors"
"github.com/gravitee-io/gravitee-kubernetes-operator/test/internal/integration/assert"
"github.com/gravitee-io/gravitee-kubernetes-operator/test/internal/integration/constants"
"github.com/gravitee-io/gravitee-kubernetes-operator/test/internal/integration/fixture"
"github.com/gravitee-io/gravitee-kubernetes-operator/test/internal/integration/labels"
Expand All @@ -28,33 +27,29 @@ import (
)

var _ = Describe("Validate update", labels.WithContext, func() {
interval := constants.Interval
ctx := context.Background()
admissionCtrl := v4.AdmissionCtrl{}

It("should return error on API update with no plans and state STARTED", func() {
fixtures := fixture.
Builder().
WithAPIv4(constants.ApiV4).
Build().
Apply()
Build()

By("removing existing plans")

clear(fixtures.APIv4.Spec.Plans)

By("checking that API validation returns errors")
Eventually(func() error {
_, err := admissionCtrl.ValidateUpdate(ctx, fixtures.APIv4, fixtures.APIv4)

return assert.Equals(
"severe",
errors.NewSeveref("cannot apply API [%s]. Its state is set to STARTED"+
" but the API has no plans. APIs must have at least one plan in order to"+
" be deployed.",
fixtures.APIv4.Name,
).Error(),
err.Error(),
)
}, constants.EventualTimeout, interval).Should(Succeed())

_, err := admissionCtrl.ValidateUpdate(ctx, fixtures.APIv4, fixtures.APIv4)

Expect(err).To(Equal(
errors.NewSeveref("cannot apply API [%s]. Its state is set to STARTED, "+
"but the API has no plans. APIs must have at least one plan in order to "+
"be deployed.",
fixtures.APIv4.Name,
),
))
})
})

0 comments on commit e91186b

Please sign in to comment.