switch process to produce groupified openshift api resources #19458
Conversation
openshift-ci-robot
added
sig/master
approved
openshift-ci-robot
added
the
size/L
deads2k
force-pushed
the
server-26-process
branch
from
920de69
to
76ad6bb
Compare
|
lgtm i guess? |
A ringing endorsement |
|
Shockingly small and an essential part of bounding inputs to transition to groupified APIs. Will take a look on Monday. Seems like we might want more tests around this. |
|
It's more a lack of endorsement of my ability to review it meaningfully. |
In defense of the existing test I had to update, there is a fair amount of variety in the template itself. |
| @@ -182,3 +181,41 @@ func (r *TemplateFileSearcher) Search(precise bool, terms ...string) (ComponentM | |||
|
|
|||
| return matches, errs | |||
| } | |||
|
|
|||
| // TemplateReference points to a stored template | |||
| @@ -192,3 +191,24 @@ func (statusStrategy) Canonicalize(obj runtime.Object) { | |||
| func (statusStrategy) ValidateUpdate(ctx apirequest.Context, obj, old runtime.Object) field.ErrorList { | |||
| return validation.ValidateTemplateInstanceUpdate(obj.(*templateapi.TemplateInstance), old.(*templateapi.TemplateInstance)) | |||
| } | |||
|
|
|||
| // ConvertUserToTemplateInstanceRequester copies analogous fields from user.Info to TemplateInstanceRequester | |||
|
|
||
| import ( | ||
| "fmt" | ||
| "regexp" | ||
| "strings" | ||
|
|
||
| "github.com/openshift/origin/pkg/api/legacygroupification" |
There was a problem hiding this comment.
nit: move this to openshift imports
| @@ -84,6 +85,10 @@ func (p *Processor) Process(template *templateapi.Template) field.ErrorList { | |||
| // referenced namespace. | |||
| stripNamespace(item) | |||
|
|
|||
| gvk := item.GetObjectKind().GroupVersionKind() | |||
There was a problem hiding this comment.
probably worth a comment about transformation from non-grouped to grouped?
| @@ -84,6 +85,10 @@ func (p *Processor) Process(template *templateapi.Template) field.ErrorList { | |||
| // referenced namespace. | |||
| stripNamespace(item) | |||
|
|
|||
| gvk := item.GetObjectKind().GroupVersionKind() | |||
| legacygroupification.OAPIToGroupifiedGVK(&gvk) | |||
| item.GetObjectKind().SetGroupVersionKind(gvk) | |||
There was a problem hiding this comment.
is it possible to do this only when we detect the non-groupified items?
There was a problem hiding this comment.
is it possible to do this only when we detect the non-groupified items?
It's a no-op otherwise. I don't like branches.
deads2k
force-pushed
the
server-26-process
branch
from
76ad6bb
to
a055828
Compare
|
nits addressed |
|
/retest |
2 similar comments
|
/retest |
|
/retest |
|
/lgtm |
|
/lgtm do we have a place to enumerate remaining oapi references we need to deal with? (ownerReferences, HPA scaleRef, etc) |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bparees, deads2k, liggitt The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
No, we don't. Suggestions? There's no place in code people will remember. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
2 similar comments
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
This updates the process api endpoint (and command) to groupify the openshift api resources before responding. I also found single-use helpers in a non-leaf package with shared helpers. I separated the two and made a leaf package for the shared bits.
@bparees @spadgett you both asked about this
@liggitt @smarterclayton we've spoken about this. I think it's a good idea.
@openshift/sig-master