Skip to content

Commit 45993dd

Browse files
a-cordiera-cordier
committed
feat: plug APIM v4 validation to admission controller
see https://gravitee.atlassian.net/browse/GKO-286
1 parent 38775ae commit 45993dd

27 files changed

+591
-32
lines changed

‎api/model/api/v4/status.go

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,28 @@
1414

1515
package v4
1616

17-
import "github.com/gravitee-io/gravitee-kubernetes-operator/api/model/api/base"
17+
import (
18+
"github.com/gravitee-io/gravitee-kubernetes-operator/api/model/api/base"
19+
)
1820

1921
type Status struct {
2022
base.Status `json:",inline"`
2123
// This field is used to store the list of plans that have been created
2224
// for the API definition if a management context has been defined
2325
// to sync the API with an APIM instance
2426
Plans map[string]string `json:"plans,omitempty"`
27+
// When API has been created regardless of errors, this field is
28+
// used to persist the error message encountered during admission
29+
Errors Errors `json:"errors,omitempty"`
30+
}
31+
32+
type Errors struct {
33+
// warning errors do not block object reconciliation,
34+
// most of the time because the value is ignored or defaulted
35+
// when the API gets synced with APIM
36+
Warning []string `json:"warning,omitempty"`
37+
// severe errors do not pass admission and will block reconcile
38+
// hence, this field should always be during the admission phase
39+
// and is very unlikely to be persisted in the status
40+
Severe []string `json:"severe,omitempty"`
2541
}

‎api/model/api/v4/zz_generated.deepcopy.go

Lines changed: 26 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎controllers/apim/apidefinition/internal/update.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ func createOrUpdateV4(ctx context.Context, apiDefinition *v1alpha1.ApiV4Definiti
147147
return err
148148
}
149149
spec.ID = cp.PickID(apim.Context)
150-
status, err := apim.APIs.ImportV4(&spec.Api)
150+
status, err := apim.APIs.ImportV4(spec)
151151
if err != nil {
152152
return err
153153
}

‎docs/api/reference.md

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7091,6 +7091,14 @@ ApiV4DefinitionStatus defines the observed state of API Definition.
70917091
The environment ID, if a management context has been defined to sync with an APIM instance<br/>
70927092
</td>
70937093
<td>false</td>
7094+
</tr><tr>
7095+
<td><b><a href="#apiv4definitionstatuserrors">errors</a></b></td>
7096+
<td>object</td>
7097+
<td>
7098+
When API has been created regardless of errors, this field is
7099+
used to persist the error message encountered during admission<br/>
7100+
</td>
7101+
<td>false</td>
70947102
</tr><tr>
70957103
<td><b>id</b></td>
70967104
<td>string</td>
@@ -7142,6 +7150,45 @@ to sync the API with an APIM instance<br/>
71427150
</tr></tbody>
71437151
</table>
71447152

7153+
7154+
### ApiV4Definition.status.errors
7155+
[Go to parent definition](#apiv4definitionstatus)
7156+
7157+
7158+
7159+
When API has been created regardless of errors, this field is
7160+
used to persist the error message encountered during admission
7161+
7162+
<table>
7163+
<thead>
7164+
<tr>
7165+
<th>Name</th>
7166+
<th>Type</th>
7167+
<th>Description</th>
7168+
<th>Required</th>
7169+
</tr>
7170+
</thead>
7171+
<tbody><tr>
7172+
<td><b>severe</b></td>
7173+
<td>[]string</td>
7174+
<td>
7175+
severe errors do not pass admission and will block reconcile
7176+
hence, this field should always be during the admission phase
7177+
and is very unlikely to be persisted in the status<br/>
7178+
</td>
7179+
<td>false</td>
7180+
</tr><tr>
7181+
<td><b>warning</b></td>
7182+
<td>[]string</td>
7183+
<td>
7184+
warning errors do not block object reconciliation,
7185+
most of the time because the value is ignored or defaulted
7186+
when the API gets synced with APIM<br/>
7187+
</td>
7188+
<td>false</td>
7189+
</tr></tbody>
7190+
</table>
7191+
71457192
## ApiResource
71467193

71477194
[gravitee.io/v1alpha1](#graviteeiov1alpha1)

‎helm/gko/crds/gravitee.io_apiv4definitions.yaml

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1278,6 +1278,28 @@ spec:
12781278
description: The environment ID, if a management context has been
12791279
defined to sync with an APIM instance
12801280
type: string
1281+
errors:
1282+
description: |-
1283+
When API has been created regardless of errors, this field is
1284+
used to persist the error message encountered during admission
1285+
properties:
1286+
severe:
1287+
description: |-
1288+
severe errors do not pass admission and will block reconcile
1289+
hence, this field should always be during the admission phase
1290+
and is very unlikely to be persisted in the status
1291+
items:
1292+
type: string
1293+
type: array
1294+
warning:
1295+
description: |-
1296+
warning errors do not block object reconciliation,
1297+
most of the time because the value is ignored or defaulted
1298+
when the API gets synced with APIM
1299+
items:
1300+
type: string
1301+
type: array
1302+
type: object
12811303
id:
12821304
description: The ID of the API definition in the Gravitee API Management
12831305
instance (if an API context has been configured).

‎internal/admission/api/v4/validate.go

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020

2121
v4 "github.com/gravitee-io/gravitee-kubernetes-operator/api/model/api/v4"
2222
"github.com/gravitee-io/gravitee-kubernetes-operator/internal/admission/api/base"
23+
"github.com/gravitee-io/gravitee-kubernetes-operator/internal/apim"
2324
"github.com/gravitee-io/gravitee-kubernetes-operator/internal/env"
2425
"github.com/gravitee-io/gravitee-kubernetes-operator/internal/errors"
2526
"github.com/gravitee-io/gravitee-kubernetes-operator/internal/k8s/dynamic"
@@ -33,18 +34,23 @@ import (
3334

3435
func validateCreate(ctx context.Context, obj runtime.Object) *errors.AdmissionErrors {
3536
errs := errors.NewAdmissionErrors()
36-
3737
if api, ok := obj.(custom.ApiDefinitionResource); ok {
3838
errs = errs.MergeWith(base.ValidateCreate(ctx, obj))
3939
if errs.IsSevere() {
4040
return errs
4141
}
4242
errs.Add(validateNoConflictingPath(ctx, api))
43+
if errs.IsSevere() {
44+
return errs
45+
}
46+
if api.HasContext() {
47+
errs = errs.MergeWith(validateDryRun(ctx, api))
48+
}
4349
}
44-
4550
return errs
4651
}
4752

53+
// TODO this should be move to base once implemented for v2
4854
func validateNoConflictingPath(ctx context.Context, api custom.ApiDefinitionResource) *errors.AdmissionError {
4955
apiPaths, err := api.GetContextPaths()
5056
if err != nil {
@@ -65,6 +71,30 @@ func validateNoConflictingPath(ctx context.Context, api custom.ApiDefinitionReso
6571
return nil
6672
}
6773

74+
func validateDryRun(ctx context.Context, api custom.ApiDefinitionResource) *errors.AdmissionErrors {
75+
errs := errors.NewAdmissionErrors()
76+
77+
apim, err := apim.FromContextRef(ctx, api.ContextRef(), api.GetNamespace())
78+
if err != nil {
79+
errs.AddSevere(err.Error())
80+
}
81+
status, err := apim.APIs.DryRunImportV4(api.GetSpec())
82+
if err != nil {
83+
errs.AddSevere(err.Error())
84+
return errs
85+
}
86+
for _, severe := range status.Errors.Severe {
87+
errs.AddSevere(severe)
88+
}
89+
if errs.IsSevere() {
90+
return errs
91+
}
92+
for _, warning := range status.Errors.Warning {
93+
errs.AddWarning(warning)
94+
}
95+
return errs
96+
}
97+
6898
func getExistingPaths(ctx context.Context, api custom.ApiDefinitionResource) ([]string, error) {
6999
existingPaths := make([]string, 0)
70100
unstructuredList, err := getListOfExistingApis(ctx, api.GetNamespace())

‎internal/admission/mctx/ctrl.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ func (a AdmissionCtrl) ValidateCreate(
4848
ctx context.Context,
4949
obj runtime.Object,
5050
) (admission.Warnings, error) {
51-
return validate(ctx, obj).Map()
51+
return validateCreate(ctx, obj).Map()
5252
}
5353

5454
// ValidateDelete implements admission.CustomValidator.

‎internal/admission/mctx/validate.go

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import (
2424
"k8s.io/apimachinery/pkg/runtime"
2525
)
2626

27-
func validate(ctx context.Context, obj runtime.Object) *errors.AdmissionErrors {
27+
func validateCreate(ctx context.Context, obj runtime.Object) *errors.AdmissionErrors {
2828
errs := errors.NewAdmissionErrors()
2929

3030
if context, ok := obj.(custom.ContextResource); ok {
@@ -54,7 +54,7 @@ func validateContextIsAvailable(ctx context.Context, context custom.ContextResou
5454
}
5555
_, err = apim.Env.Get()
5656

57-
if err != nil {
57+
if errors.IsNetworkError(err) {
5858
return errors.NewWarning(
5959
"unable to reach APIM, [%s] is not available",
6060
apim.Context.GetURL(),
@@ -68,12 +68,5 @@ func validateContextIsAvailable(ctx context.Context, context custom.ContextResou
6868
)
6969
}
7070

71-
if errors.IsNotFound(err) {
72-
return errors.NewSevere(
73-
"environment [%s/%s] could not be found in APIM",
74-
apim.Context.GetOrg(), apim.Context.GetEnv(),
75-
)
76-
}
77-
78-
return nil
71+
return errors.NewSevere(err.Error())
7972
}

‎internal/apim/service/apis.go

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,10 @@ package service
1616

1717
import (
1818
"net/http"
19+
"strconv"
1920

2021
v4 "github.com/gravitee-io/gravitee-kubernetes-operator/api/model/api/v4"
22+
"github.com/gravitee-io/gravitee-kubernetes-operator/pkg/types/k8s/custom"
2123

2224
v2 "github.com/gravitee-io/gravitee-kubernetes-operator/api/model/api/v2"
2325
"github.com/gravitee-io/gravitee-kubernetes-operator/internal/apim/client"
@@ -102,8 +104,16 @@ func (svc *APIs) ImportV2(method string, spec *v2.Api) (*model.ApiEntity, error)
102104
return api, nil
103105
}
104106

105-
func (svc *APIs) ImportV4(spec *v4.Api) (*v4.Status, error) {
106-
url := svc.EnvV2Target("apis/_import/crd")
107+
func (svc *APIs) ImportV4(spec custom.Spec) (*v4.Status, error) {
108+
return svc.importV4(spec, false)
109+
}
110+
111+
func (svc *APIs) DryRunImportV4(spec custom.Spec) (*v4.Status, error) {
112+
return svc.importV4(spec, true)
113+
}
114+
115+
func (svc *APIs) importV4(spec custom.Spec, dryRun bool) (*v4.Status, error) {
116+
url := svc.EnvV2Target("apis/_import/crd").WithQueryParam("dryRun", strconv.FormatBool(dryRun))
107117

108118
status := new(v4.Status)
109119
if err := svc.HTTP.Put(url.String(), spec, status); err != nil {

‎internal/errors/admission.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,8 @@ type AdmissionError struct {
3333
}
3434

3535
type AdmissionErrors struct {
36-
Warning []*AdmissionError `json:"warning"`
37-
Severe []*AdmissionError `json:"severe"`
36+
Warning []*AdmissionError
37+
Severe []*AdmissionError
3838
}
3939

4040
func NewAdmissionErrors() *AdmissionErrors {
@@ -56,6 +56,9 @@ func (errs *AdmissionErrors) Map() (admission.Warnings, error) {
5656
}
5757

5858
func (errs *AdmissionErrors) MergeWith(other *AdmissionErrors) *AdmissionErrors {
59+
if other == nil {
60+
return errs
61+
}
5962
errs.Warning = append(errs.Warning, other.Warning...)
6063
errs.Severe = append(errs.Severe, other.Severe...)
6164
return errs

‎internal/errors/errors.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"errors"
2020
"fmt"
2121
"io"
22+
"net"
2223
"net/http"
2324

2425
kErrors "k8s.io/apimachinery/pkg/util/errors"
@@ -129,6 +130,19 @@ func IsNotFound(err error) bool {
129130
return false
130131
}
131132

133+
func IsBadRequest(err error) bool {
134+
serverError := &ServerError{}
135+
if errors.As(err, serverError) {
136+
return serverError.StatusCode == http.StatusBadRequest
137+
}
138+
return false
139+
}
140+
141+
func IsNetworkError(err error) bool {
142+
opErr := new(net.OpError)
143+
return errors.As(err, &opErr)
144+
}
145+
132146
func IsUnauthorized(err error) bool {
133147
serverError := &ServerError{}
134148
if errors.As(err, serverError) {

0 commit comments

Comments
 (0)