Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CVE pipeline] Could we use RELATED_IMAGES_* env vars in registries and operator metadata CSV? #16815

Closed
nickboldt opened this issue Apr 30, 2020 · 15 comments
Assignees
Labels
area/che-operator Issues and PRs related to Eclipse Che Kubernetes Operator kind/enhancement A feature request - must adhere to the feature request template. severity/P1 Has a major impact to usage or development of the system.
Milestone

Comments

@nickboldt
Copy link
Contributor

nickboldt commented Apr 30, 2020

Is your enhancement related to a problem? Please describe.

This is an issue related to openshift restricted environments >= 4.3, which requires that the CSV include a relatedImages section listing the digests of all the images in the operator's payload.

While not directly a problem (yet) for Che, this will eventually be a problem when Che starts using digests in registries and operator metadata, as new builds of containers referenced by those released registries and metadata will never be available to released operators.

So if say, the java sidecar is updated to fix a CVE, a MANUAL re-release of registries and a new CSV will be required to pull the new digest value.

If we switch to using RELATED_IMAGES_* env vars for all the places where a container image reference uses a digest, then in the downstream productization flow, we will get CVE fixes for free.

Details in

https://projects.engineering.redhat.com/browse/CLOUDBLD-26?focusedCommentId=2350684&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-2350684

Describe the solution you'd like

Rather than hardcoding image references in the registries and the operator metadata, could they all be

RELATED_IMAGES_che_server
RELATED_IMAGES_operator
RELATED_IMAGES_quarkus_sidecar
RELATED_IMAGES_machine_exec
RELATED_IMAGES_theia_endpoint
...

@nickboldt nickboldt added the kind/enhancement A feature request - must adhere to the feature request template. label Apr 30, 2020
@che-bot che-bot added the status/need-triage An issue that needs to be prioritized by the curator responsible for the triage. See https://github. label Apr 30, 2020
@tolusha tolusha added the area/che-operator Issues and PRs related to Eclipse Che Kubernetes Operator label May 1, 2020
@sleshchenko sleshchenko added the severity/P1 Has a major impact to usage or development of the system. label May 4, 2020
@ericwill ericwill removed the status/need-triage An issue that needs to be prioritized by the curator responsible for the triage. See https://github. label May 4, 2020
@tolusha
Copy link
Contributor

tolusha commented May 6, 2020

@nickboldt
Duplicates?
#14993

@nickboldt
Copy link
Contributor Author

Not a dupe since the spec has changed a little since 4.3 -> 4.5. You waited long enough, so now you can use the new 4.5+ approach for declaring related images, which will flow nicely into downstream. :D

@AndrienkoAleksandr
Copy link
Contributor

AndrienkoAleksandr commented May 20, 2020

Hello, Nick. Sorry, I want to ask some implementation details.
So, you proposed:

  1. collect all plugin and devfile images in the CSV#spec#install#relatedImages . For this we have script https://github.com/eclipse/che-operator/blob/master/olm/addDigests.sh
  2. "Alias" relatedImages using annotations and envs to the che-operator deployment CSV#spec#deployments[che-operator]. In such way we will propagate all relatedImages using env variables and annotations to the che-operator container(defined in the deployment).
  3. che-operator in the container could get env variables, filter relatedImages from env(using some prefix I guess) and include found relatedImages to corresponding containers("plugin" or "devfile" registry)
  4. devfile/plugin registry uses env variables to get image with some digest and use it in the devfile.yaml/pluginMetadata.yaml.

Is it correct plan or I misunderstood something?

@tolusha
Copy link
Contributor

tolusha commented May 20, 2020

/cc @nickboldt

@nickboldt
Copy link
Contributor Author

new details about how metadata containers in Brew can automatically generate relatedImages from annotations in csv and from env vars:

https://osbs.readthedocs.io/en/latest/users.html#replacing-pullspecs

@nickboldt
Copy link
Contributor Author

nickboldt commented May 20, 2020

I think your plan makes sense but...

step 1. the existing script does not appear to read registries, so the list of sidecars/stacks/runtimes would have to be hardcoded as envs/annotations in the operator CSV as RELATED_IMAGES_foo variables. This makes sense as a manual human step since the registries are also set up that way.

(Problem is that a new devfile or plugin added to the registry would have to submit a PR against the che-operator repo to update the latest 9.9.9 CSV. But at least ALL the images that the operator needs to know about will be in one place, making it easier potentially to do CVE respins?)

step 2. not sure what this does or why we need it?

step 3. yes, the devfile registry image refs and the plugin registry image refs should have known names so that we can refer to them in meta.yaml and devfile.yaml (assuming the devfile 2.0 spec allows this?) cc: @davidfestal

step 4. yes.

@nickboldt nickboldt changed the title Could we use RELATED_IMAGES_* env vars in registries and operator metadata CSV? [CVE pipeline] Could we use RELATED_IMAGES_* env vars in registries and operator metadata CSV? May 21, 2020
@tolusha tolusha added this to the Backlog - Deploy milestone May 27, 2020
@tolusha tolusha mentioned this issue Jun 1, 2020
34 tasks
@nickboldt
Copy link
Contributor Author

Will this refactoring be done for 7.15? if so, please set milestone. We need this for digest pinning / CVE fixes in CRW 2.3 (Che 7.16).

@AndrienkoAleksandr
Copy link
Contributor

Current state:
We have for che-operator pr eclipse-che/che-operator#274. In this pr activated including to the released version CSV file section "relatedImages". Also I applied env variables in format:

env name: RELATED_IMAGES_Image_name_Image_label_Encoded_base32_image_tag_
env value: image_with_digest_to_use

I encoded in base32 image tag, because it could contains not valid for env variable name chars.

For example:

  • for plugin registry:
 - name: RELATED_IMAGE_coder_plugin_registry_image_GIXDCNJSGMWXM43DGEXDGOBOGEWWG2DFBI______
   value: docker.io/chinodesuuu/coder@sha256:6387eee627678d36fa1313017cf5620890f25559df8b24cb53e1d5f4ab6f4726
  • for devfile registry:
- name: RELATED_IMAGE_che_quarkus_devfile_registry_image_G4XDCMZOGIFA____
  value: quay.io/eclipse/che-quarkus@sha256:12b8f37808cedd9004d1dc0489d75a5df22c914645ebb7f0e0c8de8fe2e5b3c9

I created pull requests with modification entrypoint.sh scripts for devfile and plugin registry to extract and use image digest from env variables:

eclipse-che/che-devfile-registry#244
eclipse-che/che-plugin-registry#485

@tolusha tolusha modified the milestones: Backlog - Deploy, 7.15 Jun 3, 2020
@AndrienkoAleksandr
Copy link
Contributor

Hello, @nickboldt . Do we need to replace 'next' tag to digest for che-plugin-registry for released CSV? I guess we shouldn't, because next should be experimental and always the latest. What do you think?

@AndrienkoAleksandr
Copy link
Contributor

AndrienkoAleksandr commented Jun 5, 2020

@nickboldt Sorry, Do we use https://github.com/eclipse/che-plugin-registry/blob/ba10bf6972fe4bd5e903cc24b5db66111311d66a/build/dockerfiles/rhel.Dockerfile#L78 rhel7 somewhere?
Because for upstream we provide alpine based image https://github.com/eclipse/che-plugin-registry/blob/ba10bf6972fe4bd5e903cc24b5db66111311d66a/build/dockerfiles/Dockerfile#L37
For downstream (CRW I guess) looks like we are using rhel8 https://github.com/eclipse/che-plugin-registry/blob/ba10bf6972fe4bd5e903cc24b5db66111311d66a/build/dockerfiles/rhel.Dockerfile#L80

P.S.: I asked, because for rhel7 we could install coreutils, but without base32, because available older version coreutils. For rhel8 we have base32 from the box(because newer coreutils is pre-installed).

@nickboldt
Copy link
Contributor Author

We only use rhel8 in CRW builds in Brew

But to make it easier for people upstream to build on rhel, we set this dockerfile to use older rhel7 based httpd. See comments inline in the dockerfile, lines 75-82.

If you would prefer we can switch over to a pure rhel8 solution.

@AndrienkoAleksandr
Copy link
Contributor

+1 for pure rhel8.

@AndrienkoAleksandr
Copy link
Contributor

che-dev mail list item https://www.eclipse.org/lists/che-dev/msg03794.html

@mmorhun mmorhun mentioned this issue Jun 23, 2020
14 tasks
@nickboldt
Copy link
Contributor Author

Related to this activity, see https://issues.redhat.com/browse/CRW-1026 and linked RFEs in JIRA.

@tolusha tolusha modified the milestones: 7.15, 7.16 Jun 30, 2020
@AndrienkoAleksandr
Copy link
Contributor

demo here https://www.youtube.com/watch?v=rXReQy33vIM&feature=youtu.be , it contains some extra steps needed while we have not merged pr's.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/che-operator Issues and PRs related to Eclipse Che Kubernetes Operator kind/enhancement A feature request - must adhere to the feature request template. severity/P1 Has a major impact to usage or development of the system.
Projects
None yet
Development

No branches or pull requests

6 participants