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

Try to remove wrapper images (again) #621

Closed
michaelsauter opened this issue Nov 7, 2022 · 2 comments · Fixed by #648
Closed

Try to remove wrapper images (again) #621

michaelsauter opened this issue Nov 7, 2022 · 2 comments · Fixed by #648
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@michaelsauter
Copy link
Member

At the moment, the images pushed to the GitHub registry are "wrapped", meaning they are used as FROM images in images located in the OpenShift image stream. The wrapper image exists for 3 reasons:

  1. it allows to add custom CA certs
  2. it allows to embed HTTP proxy vars
  3. it allows to download additional binaries (namely, the Aqua scanner) which are not freely distributed

For #620, but also for other reasons (simplicity, less error prone) it would be nice if we could get rid of wrapping images.

(3) Could be solved by switching from Aqua to Trivy, or, if that is not feasible short/mid-term, download the Aqua scanner during the pipeline run (and cache it for next runs)

(2) Could be solved by adding a config map with HTTP proxy env vars as keys and adding all keys of that config map to the task run. Interestingly, we do not need those env vars in our enterprise cluster at all anymore (though I am not certain how that is implemented right now)

(1) This is by far the trickiest. The problem is that a proper solution requires to run update-ca-trust, which requires root, which is not available in OpenShift. An alternative would be to try and configure things manually. The basis of any solution would be to store the cert in a secret, and then mount that secret into e.g. /etc/ssl/certs (this is e.g. described in https://paraspatidar.medium.com/add-self-signed-or-ca-root-certificate-in-kubernetes-pod-ca-root-certificate-store-cb7863cb3f87). That is already enough to make the cert available to all Go code (see https://go.dev/src/crypto/x509/root_linux.go), which is a big chunk of work. However, e.g. curl wouldn't pick up the cert, one would need to use curl --cacert /etc/ssl/certs/custom.pem. In a similar fashion, we would need to ensure that the build tasks (Python, Java, Node, ...) pick up the cert correctly and any other tools we use (buildah, cnes-report, skopeo, and so on ...) also find the right cert. This looks like a daunting task. Maybe there is also another, Kubernetes "native" way to do this. I have not found anything that fits entirely, but there is still development in that area, see e.g. https://github.com/kubernetes/enhancements/tree/master/keps/sig-auth/3257-trust-anchor-sets.

@michaelsauter michaelsauter added the question Further information is requested label Nov 7, 2022
@michaelsauter
Copy link
Member Author

michaelsauter commented Nov 16, 2022

I did some research regarding (1). It's definitely cumbersome and feels brittle so it would be good to think about how to write automated tests for this. That said, it looks doable. Some hints / links below.

Everything assumes we have the following addition in the tasks:

      volumeMounts:
        - mountPath: /etc/ssl/certs/private.pem
          name: private-ssl-cert
          readOnly: true
          subPath: foo.pem
      workingDir: $(workspaces.source.path)
  volumes:
    - name: private-ssl-cert
      secret:
        secretName: ods-private-ssl-cert
        # Mark as optional since not every installation will have a private cert.
        optional: true

Further, we narrow down the problem by focusing only on getting dependencies installed via Nexus. There is no guarantee that e.g. a client written in one of the languages can access a server using the private certificate without further configuration. I think this is acceptable.

Now, on to the details about the build tasks:

ods-gradle-toolset:

# Copy global keystone to location where we can write to.
keytool -importkeystore \
  -srckeystore $JAVA_HOME/lib/security/cacerts -destkeystore ${GRADLE_USER_HOME}/cacerts \
  -deststorepass changeit -srcstorepass changeit

# Trust private cert (mounted as volume).
keytool -importcert -noprompt -trustcacerts \
  -alias private -file /etc/ssl/certs/private.pem \
  -keystore ${GRADLE_USER_HOME}/cacerts -storepass changeit

# Configure Gradle to use the modified trust store.
echo "systemProp.javax.net.ssl.trustStore=${GRADLE_USER_HOME}/cacerts" > "${GRADLE_USER_HOME}/gradle.properties"
echo 'systemProp.javax.net.ssl.trustStorePassword=changeit' >> "${GRADLE_USER_HOME}/gradle.properties"

I verified this works in general. Since ${GRADLE_USER_HOME}/cacerts is cached one needs to cater for that, e.g. by generating an md5sum of the pem file and only creating the keystore when there is a change.

ods-python-toolset:
We already set trusted-host in https://github.com/opendevstack/ods-pipeline/blob/master/build/package/scripts/build-python.sh#L65. We may need to pair this with specifying --cacert, see https://stackoverflow.com/a/26062583.

ods-npm-toolset:
It looks like https://nodejs.org/api/cli.html#node_extra_ca_certsfile and https://docs.npmjs.com/cli/v9/using-npm/config#cafile are the relevant knobs to turn.

FYI @henrjk @gerardcl we discussed this issue last Friday. What are your thoughts on this?

@henrjk
Copy link
Member

henrjk commented Nov 28, 2022

Thanks for the research and I also find this a good tradeoff. I like that it is less magic and of course that the need to have extra images would go away.
One would need to check for each language whether this works and document. For example in Python one could use https://superuser.com/a/1394302 so that one can do this at pip config time (optionally if certs are provided). I am not sure but would hope that the pip3 config set global.trusted-host "${NEXUS_HOST}" you mentioned could be removed once the certs are properly in place.

@michaelsauter michaelsauter added enhancement New feature or request and removed question Further information is requested labels Jan 6, 2023
@michaelsauter michaelsauter self-assigned this Jan 6, 2023
@michaelsauter michaelsauter added this to the 0.9.0 milestone Jan 6, 2023
michaelsauter added a commit that referenced this issue Jan 12, 2023
Supply private certificate via secret volume to tasks.

Closes #621.
michaelsauter added a commit that referenced this issue Jan 12, 2023
Supply private certificate via secret volume to tasks.

Closes #621.
michaelsauter added a commit that referenced this issue Jan 13, 2023
Supply private certificate via secret volume to tasks.

Closes #621.
michaelsauter added a commit that referenced this issue Jan 13, 2023
Supply private certificate via secret volume to tasks.

Closes #621.
michaelsauter added a commit that referenced this issue Jan 13, 2023
Supply private certificate via secret volume to tasks.

Closes #621.
michaelsauter added a commit that referenced this issue Jan 18, 2023
Supply private certificate via secret volume to tasks.

Closes #621.
michaelsauter added a commit that referenced this issue Jan 18, 2023
Supply private certificate via secret volume to tasks.

Closes #621.
michaelsauter added a commit that referenced this issue Jan 18, 2023
Supply private certificate via secret volume to tasks.

Closes #621.
michaelsauter added a commit that referenced this issue Jan 18, 2023
Supply private certificate via secret volume to tasks.

Closes #621.
michaelsauter added a commit that referenced this issue Jan 27, 2023
Supply private certificate via secret volume to tasks.

Closes #621.
michaelsauter added a commit that referenced this issue Jan 30, 2023
Supply private certificate via secret volume to tasks.

Closes #621.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants