SHIP-0027: Image Abstraction Layer #57
Conversation
| | ---------- | --------------------------------------------------------------------------------- | | ||
| | from | Indicates the source of the image (from where Tagger should import it) | | ||
| | mirror | Informs if the Image should be mirrored to another registry | | ||
| | insecure | Indicates that Tagger should skip tls verification during the image import/mirror | |
There was a problem hiding this comment.
Well, you spotted a place where copy & pasta failed me miserably. I have already adjusted these markdown tables (I have copied them from the PoC repository).
When it comes to you actual question: I don't know. When you say "builders" what exactly do you mean ?
There was a problem hiding this comment.
I haven't found anything here:
https://pkg.go.dev/github.com/shipwright-io/build/pkg/apis/build/v1alpha1#Image
| All Image properties also show up in an ImageImport object, so, if the user does not provide data | ||
| for these properties in the ImageImport the operator will use whatever is set in the Image instead. | ||
| Follow a list of meaning for each property in an ImageImport.Spec: |
There was a problem hiding this comment.
Does that make sense ? Would either targetImage or the other three properties be a reasonable alternative?
There was a problem hiding this comment.
I am seeing this as: an Image object holds "defaults" and these may be overwritten by the user in a per import request basis. That can also be useful when creating an Image object based solely in an ImageImport. That is why the properties are replicated in the Image and in the ImageImport. WDYT ?
|
|
||
| | Name | Description | | ||
| | -------------- | ------------ | | ||
| | from | Keeps a reference from where the image got imported | |
There was a problem hiding this comment.
Will this include the digest?
There was a problem hiding this comment.
No, what contains the digest is the imageReference property below. As the Image may have its "From" property changed in time it is useful, IMHO, to have recorded what the "From" was at the moment of import. WDYT?
| | Name | Description | | ||
| | ------- | ------------------------------------------------------------------------------------ | | ||
| | when | Date and time of the import attempt | | ||
| | succeed | A boolean indicating if the import was successful or not | |
There was a problem hiding this comment.
Would this be rather a status (True, False, Unknown)?
There was a problem hiding this comment.
Yes ! You are absolutely right ! This should be a Condition inside the Status. I am going to address it.
There was a problem hiding this comment.
I have adjusted this EP to include a "Condition" property in the ImageImport status. PTAL.
| To import images hosted in registries protected by authentication the operator would need to be | ||
| able to read docker credentials secrets that exist in the same namespace where the Image exists. | ||
| The idea is to mimic OpenShift’s Image Stream here: read all docker config secrets from the target | ||
| namespace and use the appropriate one (if present). |
There was a problem hiding this comment.
I see, related to my point above. I am not sure if this implicity is a good idea. One can easily have multiple secrets for the same host that have access to different repositories under that container registry host. Though, not sure if that is realistic inside a single Kubernetes namespace.
There was a problem hiding this comment.
Not sure where your concern is, if I understand you right then we are covered by a keyring as used by k8s itself, no? Can you think in a scenario where such a keyring would fall short?
ricardomaraschini
changed the title
|
|
||
| ### Risks and Mitigations | ||
|
|
||
| **Confusion with OpenShift’s ImageStream implementation** |
There was a problem hiding this comment.
In my view this EP clearly explains the the relationship, image is inspired but meant to work against any Kubernetes cluster, like all other Shipwright components. Also, we focus on the features that are most important to this organization, instead of trying to pursue a drop-in replacement.
There was a problem hiding this comment.
IIUC we would not need to worry about this naming confusion then, is that what you mean?
There was a problem hiding this comment.
@ricardomaraschini very neat, nice work!. Adding my comments, pls take a look.
There was a problem hiding this comment.
@ricardomaraschini hi, from my side just few comments left, it looks good.
There was a problem hiding this comment.
@ricardomaraschini approving from my side, nice contributions! Thanks
/lgtm
openshift-ci
bot
added
lgtm
|
@sbose78 did you have time to go through this ? |
Proposal for a Kubernetes Image Abstraction Layer similar to OpenShift's ImageStreams.
done. |
|
/lgtm |
There was a problem hiding this comment.
/approve
I think this looks good - I have a note regarding the ability to specify multiple mirrors. But (a) I am not sure if this is a long-term requirement, and (b) it should not block this EP, as we can make breaking changes to the API at this phase of the project.
| namespace: my-namespace | ||
| spec: | ||
| source: quay.io/company/myapp:latest | ||
| mirror: false |
There was a problem hiding this comment.
Something to consider for the future - having the ability to specify mirrors on a per-image basis.
The current design implies that there is a single mirror registry that is centrally configured. Having the ability to specify the mirror configuration directly for an image (host, auth, etc.) may be useful.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: adambkaplan 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 |
openshift-ci
bot
added
the
approved
|
@adambkaplan Thanks for approving this. @imjasonh @qu1queee @otaviof @SaschaSchwarze0 During the next couple of days I will be opening a few issues in the shipwright-io/image repository to adjust items that are in this EP and are not yet implemented. Thank you all. |
Proposal for a Kubernetes Image Abstraction Layer similar to OpenShift's
ImageStreams.