Skip to content

Commit 84df511

Browse files
committed
fix: populate API ids before runing a dry run
1 parent e77f64a commit 84df511

File tree

11 files changed

+123
-115
lines changed

11 files changed

+123
-115
lines changed

api/model/management/context.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222

2323
var _ custom.Auth = &Auth{}
2424
var _ custom.BasicAuth = &BasicAuth{}
25+
var _ custom.Context = &Context{}
2526

2627
type Context struct {
2728
// The URL of a management API instance

api/v1alpha1/apiv2definition_types.go

Lines changed: 74 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ type ApiDefinitionStatus struct {
6060
base.Status `json:",inline"`
6161
}
6262

63-
var _ custom.ApiDefinition = &ApiDefinition{}
63+
var _ custom.ApiDefinitionResource = &ApiDefinition{}
6464
var _ custom.Status = &ApiDefinitionStatus{}
6565
var _ custom.Spec = &ApiDefinitionV2Spec{}
6666

@@ -81,39 +81,6 @@ type ApiDefinition struct {
8181
Status ApiDefinitionStatus `json:"status,omitempty"`
8282
}
8383

84-
// PickID returns the ID of the API definition, when a context has been defined at the spec level.
85-
// The ID might be returned from the API status, meaning that the API is already known.
86-
// If the API is unknown, the ID is either given from the spec if given,
87-
// or generated from the API UID and the context key to ensure uniqueness
88-
// in case the API is replicated on a same APIM instance.
89-
func (api *ApiDefinition) PickID() string {
90-
if api.Status.ID != "" {
91-
return api.Status.ID
92-
}
93-
94-
if api.Spec.ID != "" {
95-
return api.Spec.ID
96-
}
97-
98-
return string(api.UID)
99-
}
100-
101-
func (api *ApiDefinition) PickCrossID() string {
102-
if api.Status.CrossID != "" {
103-
return api.Status.CrossID
104-
}
105-
106-
return api.GetOrGenerateCrossID()
107-
}
108-
109-
func (api *ApiDefinition) GetOrGenerateCrossID() string {
110-
if api.Spec.CrossID != "" {
111-
return api.Spec.CrossID
112-
}
113-
114-
return uuid.FromStrings(api.GetNamespacedName().String())
115-
}
116-
11784
func (api *ApiDefinition) GetNamespacedName() *refs.NamespacedName {
11885
return &refs.NamespacedName{Namespace: api.Namespace, Name: api.Name}
11986
}
@@ -169,6 +136,79 @@ func (api *ApiDefinition) GetContextPaths() ([]string, error) {
169136
return api.Spec.GetContextPaths()
170137
}
171138

139+
func (api *ApiDefinition) GetDefinition() custom.ApiDefinition {
140+
return &api.Spec.Api
141+
}
142+
143+
func (api *ApiDefinition) PopulateIDs(_ custom.Context) {
144+
api.Spec.ID = api.pickID()
145+
api.Spec.CrossID = api.pickCrossID()
146+
api.generateEmptyPlanCrossIds()
147+
api.generatePageIDs()
148+
}
149+
150+
// For each plan, generate a CrossId from Api Id & Plan Name if not defined.
151+
func (api *ApiDefinition) generateEmptyPlanCrossIds() {
152+
plans := api.Spec.Plans
153+
154+
for _, plan := range plans {
155+
if plan.CrossId == "" {
156+
plan.CrossId = uuid.FromStrings(api.Spec.ID, separator, plan.Name)
157+
}
158+
}
159+
}
160+
161+
func (api *ApiDefinition) generatePageIDs() {
162+
spec := &api.Spec
163+
pages := spec.Pages
164+
for name, page := range pages {
165+
page.API = spec.ID
166+
apiName := api.GetNamespacedName().String()
167+
if page.CrossID == "" {
168+
page.CrossID = uuid.FromStrings(apiName, separator, name)
169+
}
170+
if page.ID == "" {
171+
page.ID = uuid.FromStrings(spec.ID, separator, name)
172+
}
173+
if page.Parent != "" {
174+
page.ParentID = uuid.FromStrings(spec.ID, separator, page.Parent)
175+
}
176+
}
177+
}
178+
179+
// PickID returns the ID of the API definition, when a context has been defined at the spec level.
180+
// The ID might be returned from the API status, meaning that the API is already known.
181+
// If the API is unknown, the ID is either given from the spec if given,
182+
// or generated from the API UID and the context key to ensure uniqueness
183+
// in case the API is replicated on a same APIM instance.
184+
func (api *ApiDefinition) pickID() string {
185+
if api.Status.ID != "" {
186+
return api.Status.ID
187+
}
188+
189+
if api.Spec.ID != "" {
190+
return api.Spec.ID
191+
}
192+
193+
return string(api.UID)
194+
}
195+
196+
func (api *ApiDefinition) pickCrossID() string {
197+
if api.Status.CrossID != "" {
198+
return api.Status.CrossID
199+
}
200+
201+
return api.getOrGenerateCrossID()
202+
}
203+
204+
func (api *ApiDefinition) getOrGenerateCrossID() string {
205+
if api.Spec.CrossID != "" {
206+
return api.Spec.CrossID
207+
}
208+
209+
return uuid.FromStrings(api.GetNamespacedName().String())
210+
}
211+
172212
func (spec *ApiDefinitionV2Spec) Hash() string {
173213
return hash.Calculate(spec)
174214
}

api/v1alpha1/apiv4definition_types.go

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ import (
2929
"sigs.k8s.io/controller-runtime/pkg/client"
3030
)
3131

32+
const separator = "/"
33+
3234
// ApiV4DefinitionSpec defines the desired state of ApiDefinition.
3335
// +kubebuilder:object:generate=true
3436
type ApiV4DefinitionSpec struct {
@@ -71,12 +73,19 @@ func (api *ApiV4Definition) IsBeingDeleted() bool {
7173
return !api.ObjectMeta.DeletionTimestamp.IsZero()
7274
}
7375

74-
// PickID returns the ID of the API definition, when a context has been defined at the spec level.
76+
func (api *ApiV4Definition) PopulateIDs(context custom.Context) {
77+
api.Spec.ID = api.pickID(context)
78+
api.Spec.CrossID = api.pickCrossID()
79+
api.Spec.Pages = api.pickPageIDs()
80+
api.Spec.Plans = api.pickPlanIDs()
81+
}
82+
83+
// pickID returns the ID of the API definition, when a context has been defined at the spec level.
7584
// The ID might be returned from the API status, meaning that the API is already known.
7685
// If the API is unknown, the ID is either given from the spec if given,
7786
// or generated from the API UID and the context key to ensure uniqueness
7887
// in case the API is replicated on a same APIM instance.
79-
func (api *ApiV4Definition) PickID(mCtx custom.Context) string {
88+
func (api *ApiV4Definition) pickID(mCtx custom.Context) string {
8089
if api.Status.ID != "" {
8190
return api.Status.ID
8291
}
@@ -86,13 +95,13 @@ func (api *ApiV4Definition) PickID(mCtx custom.Context) string {
8695
}
8796

8897
if mCtx != nil {
89-
return uuid.FromStrings(api.PickCrossID(), mCtx.GetOrg(), mCtx.GetEnv())
98+
return uuid.FromStrings(api.pickCrossID(), mCtx.GetOrg(), mCtx.GetEnv())
9099
}
91100

92101
return string(api.UID)
93102
}
94103

95-
func (api *ApiV4Definition) PickCrossID() string {
104+
func (api *ApiV4Definition) pickCrossID() string {
96105
if api.Status.CrossID != "" {
97106
return api.Status.CrossID
98107
}
@@ -105,7 +114,7 @@ func (api *ApiV4Definition) PickCrossID() string {
105114
return uuid.FromStrings(namespacedName.String())
106115
}
107116

108-
func (api *ApiV4Definition) PickPlanIDs() map[string]*v4.Plan {
117+
func (api *ApiV4Definition) pickPlanIDs() map[string]*v4.Plan {
109118
plans := make(map[string]*v4.Plan, len(api.Spec.Plans))
110119
for key, plan := range api.Spec.Plans {
111120
p := plan.DeepCopy()
@@ -120,18 +129,7 @@ func (api *ApiV4Definition) PickPlanIDs() map[string]*v4.Plan {
120129
return plans
121130
}
122131

123-
const separator = "/"
124-
125-
// GetOrGenerateEmptyPlanCrossID For each plan, generate a CrossId from Api Id & Plan Name if not defined.
126-
func (api *ApiV4Definition) GetOrGenerateEmptyPlanCrossID() {
127-
for name, plan := range api.Spec.Plans {
128-
if plan.CrossId == "" {
129-
plan.CrossId = uuid.FromStrings(api.PickCrossID(), separator, name)
130-
}
131-
}
132-
}
133-
134-
func (api *ApiV4Definition) PickPageIDs() map[string]*v4.Page {
132+
func (api *ApiV4Definition) pickPageIDs() map[string]*v4.Page {
135133
pages := make(map[string]*v4.Page, len(api.Spec.Pages))
136134
for name, page := range api.Spec.Pages {
137135
p := page.DeepCopy()
@@ -208,6 +206,10 @@ func (api *ApiV4Definition) GetDefinitionVersion() custom.ApiDefinitionVersion {
208206
return custom.ApiV4
209207
}
210208

209+
func (api *ApiV4Definition) GetDefinition() custom.ApiDefinition {
210+
return &api.Spec.Api
211+
}
212+
211213
func (spec *ApiV4DefinitionSpec) Hash() string {
212214
return hash.Calculate(spec)
213215
}

controllers/apim/apidefinition/internal/helper.go

Lines changed: 0 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -17,40 +17,8 @@ package internal
1717
import (
1818
"github.com/gravitee-io/gravitee-kubernetes-operator/api/v1alpha1"
1919
apimModel "github.com/gravitee-io/gravitee-kubernetes-operator/internal/apim/model"
20-
"github.com/gravitee-io/gravitee-kubernetes-operator/internal/uuid"
2120
)
2221

23-
const separator = "/"
24-
25-
// For each plan, generate a CrossId from Api Id & Plan Name if not defined.
26-
func generateEmptyPlanCrossIds(spec *v1alpha1.ApiDefinitionV2Spec) {
27-
plans := spec.Plans
28-
29-
for _, plan := range plans {
30-
if plan.CrossId == "" {
31-
plan.CrossId = uuid.FromStrings(spec.ID, separator, plan.Name)
32-
}
33-
}
34-
}
35-
36-
func generatePageIDs(api *v1alpha1.ApiDefinition) {
37-
spec := &api.Spec
38-
pages := spec.Pages
39-
for name, page := range pages {
40-
page.API = spec.ID
41-
apiName := api.GetNamespacedName().String()
42-
if page.CrossID == "" {
43-
page.CrossID = uuid.FromStrings(apiName, separator, name)
44-
}
45-
if page.ID == "" {
46-
page.ID = uuid.FromStrings(spec.ID, separator, name)
47-
}
48-
if page.Parent != "" {
49-
page.ParentID = uuid.FromStrings(spec.ID, separator, page.Parent)
50-
}
51-
}
52-
}
53-
5422
// Retrieve the plan ids from the management apiEntity.
5523
func retrieveMgmtPlanIds(spec *v1alpha1.ApiDefinitionV2Spec, mgmtApi *apimModel.ApiEntity) {
5624
plans := spec.Plans

controllers/apim/apidefinition/internal/update.go

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -49,14 +49,14 @@ func createOrUpdateV2(ctx context.Context, apiDefinition *v1alpha1.ApiDefinition
4949
spec := &cp.Spec
5050
formerStatus := cp.Status
5151

52-
spec.ID = cp.PickID()
5352
spec.SetDefinitionContext()
54-
generateEmptyPlanCrossIds(spec)
5553

5654
if err := resolveResources(ctx, spec.Resources); err != nil {
5755
return err
5856
}
5957

58+
cp.PopulateIDs(nil)
59+
6060
if !apiDefinition.HasContext() {
6161
if !spec.IsLocal {
6262
return errors.NewUnrecoverableError("a context is required when setting local to false")
@@ -76,9 +76,6 @@ func createOrUpdateV2(ctx context.Context, apiDefinition *v1alpha1.ApiDefinition
7676
return apimErr
7777
}
7878

79-
generatePageIDs(cp)
80-
spec.CrossID = cp.PickCrossID()
81-
8279
_, findErr := apim.APIs.GetByCrossID(spec.CrossID)
8380
if errors.IgnoreNotFound(findErr) != nil {
8481
return errors.NewContextError(findErr)
@@ -135,9 +132,6 @@ func createOrUpdateV4(ctx context.Context, apiDefinition *v1alpha1.ApiV4Definiti
135132
return err
136133
}
137134

138-
spec.CrossID = cp.PickCrossID()
139-
spec.Plans = cp.PickPlanIDs()
140-
spec.Pages = cp.PickPageIDs()
141135
spec.DefinitionContext = v4.NewDefaultKubernetesContext().MergeWith(spec.DefinitionContext)
142136

143137
if spec.Context != nil {
@@ -146,15 +140,15 @@ func createOrUpdateV4(ctx context.Context, apiDefinition *v1alpha1.ApiV4Definiti
146140
if err != nil {
147141
return err
148142
}
149-
spec.ID = cp.PickID(apim.Context)
150-
status, err := apim.APIs.ImportV4(spec)
143+
cp.PopulateIDs(apim.Context)
144+
status, err := apim.APIs.ImportV4(&spec.Api)
151145
if err != nil {
152146
return err
153147
}
154148
apiDefinition.Status.Status = *status
155149
log.FromContext(ctx).WithValues("id", spec.ID).Info("API successfully synced with APIM")
156150
} else {
157-
spec.ID = cp.PickID(nil)
151+
cp.PopulateIDs(nil)
158152
}
159153

160154
if spec.DefinitionContext.SyncFrom == v4.OriginManagement || spec.State == base.StateStopped {

internal/admission/api/v4/validate.go

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,11 +74,21 @@ func validateNoConflictingPath(ctx context.Context, api custom.ApiDefinitionReso
7474
func validateDryRun(ctx context.Context, api custom.ApiDefinitionResource) *errors.AdmissionErrors {
7575
errs := errors.NewAdmissionErrors()
7676

77-
apim, err := apim.FromContextRef(ctx, api.ContextRef(), api.GetNamespace())
77+
cp, _ := api.DeepCopyResource().(custom.ApiDefinitionResource)
78+
79+
apim, err := apim.FromContextRef(ctx, cp.ContextRef(), cp.GetNamespace())
7880
if err != nil {
7981
errs.AddSevere(err.Error())
8082
}
81-
status, err := apim.APIs.DryRunImportV4(api.GetSpec())
83+
84+
cp.PopulateIDs(apim.Context)
85+
86+
impl, ok := cp.GetDefinition().(*v4.Api)
87+
if !ok {
88+
errs.AddSevere("unable to call dry run import because api is not a v4 API")
89+
}
90+
91+
status, err := apim.APIs.DryRunImportV4(impl)
8292
if err != nil {
8393
errs.AddSevere(err.Error())
8494
return errs

internal/admission/mctx/validate.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,5 +68,9 @@ func validateContextIsAvailable(ctx context.Context, context custom.ContextResou
6868
)
6969
}
7070

71-
return errors.NewSevere(err.Error())
71+
if errors.IsBadRequest(err) {
72+
return errors.NewSevere(err.Error())
73+
}
74+
75+
return nil
7276
}

internal/apim/service/apis.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import (
1919
"strconv"
2020

2121
v4 "github.com/gravitee-io/gravitee-kubernetes-operator/api/model/api/v4"
22-
"github.com/gravitee-io/gravitee-kubernetes-operator/pkg/types/k8s/custom"
2322

2423
v2 "github.com/gravitee-io/gravitee-kubernetes-operator/api/model/api/v2"
2524
"github.com/gravitee-io/gravitee-kubernetes-operator/internal/apim/client"
@@ -104,15 +103,15 @@ func (svc *APIs) ImportV2(method string, spec *v2.Api) (*model.ApiEntity, error)
104103
return api, nil
105104
}
106105

107-
func (svc *APIs) ImportV4(spec custom.Spec) (*v4.Status, error) {
106+
func (svc *APIs) ImportV4(spec *v4.Api) (*v4.Status, error) {
108107
return svc.importV4(spec, false)
109108
}
110109

111-
func (svc *APIs) DryRunImportV4(spec custom.Spec) (*v4.Status, error) {
110+
func (svc *APIs) DryRunImportV4(spec *v4.Api) (*v4.Status, error) {
112111
return svc.importV4(spec, true)
113112
}
114113

115-
func (svc *APIs) importV4(spec custom.Spec, dryRun bool) (*v4.Status, error) {
114+
func (svc *APIs) importV4(spec *v4.Api, dryRun bool) (*v4.Status, error) {
116115
url := svc.EnvV2Target("apis/_import/crd").WithQueryParam("dryRun", strconv.FormatBool(dryRun))
117116

118117
status := new(v4.Status)

pkg/types/k8s/custom/custom.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ type ApiDefinition interface {
5353
type ApiDefinitionResource interface {
5454
ContextAwareResource
5555
ApiDefinition
56+
GetDefinition() ApiDefinition
57+
PopulateIDs(context Context)
5658
}
5759

5860
// +k8s:deepcopy-gen=false

0 commit comments

Comments
 (0)