Migrate console extensions CRDs from console-operator#95
Conversation
openshift-ci-robot
added
the
size/XXL
Bootstrapping and installation is not the point here. All resources which are needed by other operators (especially those which come first in the installation order), must go to this repo. I understood that this is the case for the console types: kube-apiserver operator might want to install an extension for the console. It can only do that if the CRD is already installed in the system. |
|
Agree with @sttts. I thought these are just console specific CRD's to support extensions. /lgtm |
openshift-ci-robot
added
lgtm
|
Needs a rebase and CRD regeneration (we moved to 1.16). |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
10 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. |
|
/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. |
|
/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. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
benjaminapetersen
force-pushed
the
console/extensions
branch
from
5165ade to
8295ca6
Compare
|
Rebased & regeneration checks out. Should be good to go. |
|
Bump for approval again, then I can move along the removal from |
|
Ah, I see another rebase needed, fixing quick. |
| HTML | ||
| properties: | ||
| href: | ||
| description: href is the absolute secure URL for the link (must |
There was a problem hiding this comment.
you can use pattern: "<regex>" to restrict the value.
There was a problem hiding this comment.
This I need to fix as it has been part of the historical API. We have been using:
pattern: '^https://([\w-]+.)+[\w-]+(/[\w- ./?%&=])?$'
Just need to figure out how to get this in openshift/api to generate.
There was a problem hiding this comment.
Ok this should be resolved via https://github.com/openshift/api/pull/457/files#r331225859 on our next regeneration.
| description: description is the description of the CLI download (can | ||
| include markdown). | ||
| type: string | ||
| displayName: |
There was a problem hiding this comment.
It's a required field, so it should not be empty.
There was a problem hiding this comment.
Are we allowed to change this now? API has been in for a release already.
| description: href is the absolute secure URL for the link (must | ||
| use https) | ||
| type: string | ||
| text: |
There was a problem hiding this comment.
Perhaps we assumed required would validate that it is not empty. I'm guessing present but empty is allowed without regex, given your questions. We should update.
There was a problem hiding this comment.
Are we allowed to change this now? API has been in for a release already.
| RegExp constructor. If not specified, links will be displayed for | ||
| all the namespaces. | ||
| type: string | ||
| text: |
There was a problem hiding this comment.
Same, don't know if we can change it at this point.
| description: ConsoleExternalLogLinkSpec is the desired log link configuration. | ||
| The log link will appear on the logs tab of the pod details page. | ||
| properties: | ||
| hrefTemplate: |
There was a problem hiding this comment.
use pattern to restrict it to https
There was a problem hiding this comment.
https://github.com/openshift/api/pull/457/files#r331226280 should resolve on our next regeneration PR.
benjaminapetersen
force-pushed
the
console/extensions
branch
from
8295ca6 to
1658567
Compare
openshift-ci-robot
added
size/L
benjaminapetersen
force-pushed
the
console/extensions
branch
from
1658567 to
f92b24e
Compare
| @@ -0,0 +1,87 @@ | |||
| apiVersion: apiextensions.k8s.io/v1beta1 | |||
There was a problem hiding this comment.
Rebase, and I have to say I like the output of this much better. It's limited to the fields that specifically matter to the custom resource.
|
Validation as follow-up. /lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: benjaminapetersen, mfojtik, sttts The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Looks like we had a mismatch between openshift/api and what was in the CRD: Link text is now required, when it wasn't before. (Breaking the console merge queue) |
|
Console queue fixed by openshift/console#2888 |
This should not have landed in this repo. This is not about cross dependencies, this is about bootstrapping a cluster. This is not a dumping ground for, "hey something else might/does use this". |
9 participants
See openshift/console-operator#302 for associated removal PR. These files have existed in the console-operator, but should live here.
This is not a 1:1, however, due to
update-codegen-crdsmodifying the files.@spadgett @rhamilto @jhadvig fyi.
The
codegencommand overwrites these files based on what is inopenshift/api, so what we previously had documented may no longer be here (if it was also not inopenshift/api)