update image policy API for first class resolution #10451
Conversation
| @@ -13,18 +13,34 @@ const IgnorePolicyRulesAnnotation = "alpha.image.policy.openshift.io/ignore-rule | |||
| type ImagePolicyConfig struct { | |||
| unversioned.TypeMeta | |||
|
|
|||
| // ResolveImages indicates what kind of image resolution should be done. If a rewriting policy is chosen, | |||
| // then the image pull specs will be updated. | |||
| ResolveImages ImageResolutionType | |||
There was a problem hiding this comment.
@smarterclayton API changes here.
There was a problem hiding this comment.
I think I'm fine with this. Since only things that are either in the integrated registry or have a digest can be resolved, it's not like this takes options away.
deads2k
force-pushed
the
change-image-api
branch
from
5b27e5f to
d370950
Compare
|
@mfojtik can you review this one? |
|
@deads2k will tomorrow morning |
| case err != nil && imagepolicyapi.FailOnResolutionFailure(imageResolutionType): | ||
| // if we had a resolution error and we're supposed to fail, fail | ||
| decision.err = err | ||
| decision.tested = true |
There was a problem hiding this comment.
if you return at bottom, are these lost? I don't see you returning them
There was a problem hiding this comment.
if you return at bottom, are these lost? I don't see you returning them
added to cache here. The value is scoped high up, so returning down below (if we tested) will store in the cache.
|
[test] LGTM (will appreciate more tests, but we can always add them later ;-) |
This a refactor of existing types, so all the old tests continuing to pass indicates success. |
deads2k
force-pushed
the
change-image-api
branch
from
d370950 to
f0dbe67
Compare
|
Comments addressed |
|
@simon3z @moolitayer The API for the ImagePolicy admission plugin is changing before 3.3. I'm updating the doc the pull today to match this API. Image pull spec resolution is changing. |
deads2k
force-pushed
the
change-image-api
branch
from
f0dbe67 to
3507ef2
Compare
deads2k
force-pushed
the
change-image-api
branch
from
3507ef2 to
ed79340
Compare
|
[merge] |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/8229/) (Image: devenv-rhel7_4878) |
|
Evaluated for origin merge up to ed79340 |
|
Evaluated for origin test up to ed79340 |
|
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/8230/) |
Change the image policy API to resolve separately. Image resolution is logically a separate step and the resolution would be shared by every test after this. Setting it globally makes sense.
@smarterclayton I think you agreed in concept, make sure you agree in the particulars.