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

Add new Containerfile.tools to be used by openshift.release #15

Merged
merged 2 commits into from
Nov 15, 2023

Conversation

irinamihai
Copy link
Collaborator

@irinamihai irinamihai commented Nov 14, 2023

Description:

  • this new Containerfile.tests will be used by the CI/CD system to run all the needed unit tests
  • it will be used as a build_root by the ci-operator config so that we are able to use the latest golang since there is no OCP builder image for go 1.21 yet

@irinamihai irinamihai marked this pull request as draft November 14, 2023 22:48
Description:
- this new Containerfile.tests will be used by the CI/CD system
  to run all the needed unit tests
- it will be used as a build_root so that we are able to use the
  latest golang since there is no OCP builder image for go 1.21 yet
@irinamihai irinamihai force-pushed the add_tests_containerfile branch from adb782a to d2b1607 Compare November 14, 2023 22:49
# the License.
#

FROM docker.io/golang:1.21 AS builder
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use RHEL as the base image, like we do in Containerfile?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can, for sure. I wanted to get away from having to install the Go 1.21 compiler and I saw that other operators in openshift-kni are also using this image. The preferred one would be the Go 1.21 version of registry.ci.openshift.org/ocp/builder:rhel-9-golang-1.20-openshift-4.15, but there isn't one yet.


# Install golangci-lint.
RUN curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.55.2 && \
golangci-lint --version
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This way to install golangci-lint is a bit scary: it runs a shell script downloaded from GitHub that we don't really know what will do. Can we instead download the release artifact, verify that it hasn't changed since we checked the content and install it? For example:

RUN \
  curl -Lo tarball https://github.com/golangci/golangci-lint/releases/download/v1.55.2/golangci-lint-1.55.2-linux-amd64.tar.gz && \
  echo ca21c961a33be3bc15e4292dc40c98c8dcc5463a7b6768a3afc123761630c09c tarball | sha256sum -c && \
  tar -C /usr/local/bin --strip-components=1 -xf tarball golangci-lint-1.55.2-linux-amd64/golangci-lint && \
  rm tarball

The point there is that you are supposed to manually download the file, check that it is correct and then recalculate the sha256sum yourself. That way, if the file were to change (intentionally or by accident) the build will fail.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely.

Copy link
Collaborator

@jhernand jhernand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still marked as draft, but looks good to me anyhow. Thanks @irinamihai !

@irinamihai irinamihai force-pushed the add_tests_containerfile branch from 94096f4 to 6e6260c Compare November 15, 2023 17:56
@irinamihai irinamihai force-pushed the add_tests_containerfile branch from 6e6260c to 508bb6e Compare November 15, 2023 17:57
@irinamihai
Copy link
Collaborator Author

Still marked as draft, but looks good to me anyhow. Thanks @irinamihai !

@jhernand, thanks for taking a look. I wanted to have the openshift/release part in a more stable format before merging this.

@irinamihai irinamihai marked this pull request as ready for review November 15, 2023 17:59
@jhernand
Copy link
Collaborator

@jhernand, thanks for taking a look. I wanted to have the openshift/release part in a more stable format before merging this.

No problem, whatever you prefer.

@irinamihai irinamihai changed the title Add new Containerfile.tests to be used by openshift.release Add new Containerfile.tools to be used by openshift.release Nov 15, 2023
@irinamihai irinamihai merged commit b48f77e into openshift-kni:main Nov 15, 2023
4 checks passed
@irinamihai irinamihai deleted the add_tests_containerfile branch December 13, 2023 23:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants