Fix samplesRegistry replacement in imageStreams when using paths #288
Conversation
If oldreg has a registry found in oldies, replace the oldreg registry part with newreg and return it.
Interesting / thanks @aerdei ! Any chance you add a unit test that validates this change (it did pass the existing validations)? The current validations are in https://github.com/openshift/cluster-samples-operator/blob/master/pkg/stub/handler_test.go#L294, admittedly a somewhat involved flow. If you want to take it on there, great. Otherwise, a new test method which validates your scenario and simply calls |
|
/approve I'll also note there is an RFE on the books for expanding the power of this aspect to override more than just the registry host:port, but it has not been prioritized as of yet. |
openshift-ci-robot
added
the
approved
Since .spec.dockerImageRepository is deprecated, add a new IS test file with a more detailed .spec.tags.from.name to extend test scenarios.
Add test cases where samplesRegistry is of the form host/path and host:port/path.
|
Hi @gabemontero , |
|
/lgtm thanks again @aerdei |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aerdei, gabemontero 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 |
1 similar comment
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aerdei, gabemontero 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 |
|
/retest |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
3 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. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
Currently, when defining samplesRegistry in the CR with a value of newhost:1111/path, the resulting imageStream DockerImage values become newhost:1111/path/path/path instead of newhost:1111/path/oldpaths.
This PR makes it possible to have a path in samplesRegistry in the CR. This could be beneficial when using external registries with paths (e.g. Artifactory remote registries).