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

Build container with arguments #19

Merged
merged 14 commits into from
Jul 29, 2022
Merged

Conversation

damacus
Copy link
Contributor

@damacus damacus commented Jul 14, 2022

Allow container to take build args so that we can select the versions of Terraform and Ruby

  • Base the container off Ruby rather than the Terraform image so that we have all the tools we need
  • Upgrade the default Ruby version to the latest stable (3.1.2)
  • bundle exec the kitchen binary in the entrypoint now that we're bundle installing the Ruby Gems
  • Add a default source to the Gemfile, not doing so was deprecated in a recent bundler version
  • Add a Gemfile.lock so that we are able to lock the Gems

Signed-off-by: Dan Webb dan.webb@damacus.io

damacus added 2 commits July 14, 2022 13:40
Allow container to take build args so that we can select the versions of Terraform and Ruby

- Base the container off Ruby rather than the Terraform image so that we have all the tools we need
- Upgrade the default Ruby version to the latest stable (3.1.2)
- bundle exec the kitchen binary in the entrypoint now that we're bundle installing the Ruby Gems
- Add a default source to the Gemfile, not doing so was deprecated in a recent bundler version
- Add a Gemfile.lock so that we are able to lock the Gems

Signed-off-by: Dan Webb <dan.webb@damacus.io>
This command is only available to be run as root and we are running as
kitchen for security reasons

Signed-off-by: Dan Webb <dan.webb@damacus.io>
@damacus
Copy link
Contributor Author

damacus commented Jul 14, 2022

@dan-hill2802 if we want to run update-ca-certificates at runtime in the entry point, we'll need to run that under sudo or something similar. Which isn't included in the image as is.

@dan-hill2802
Copy link
Contributor

@dan-hill2802 if we want to run update-ca-certificates at runtime in the entry point, we'll need to run that under sudo or something similar. Which isn't included in the image as is.

thinking about it, we can probably just change the user to root in the docker run command whenever we need to load more certs

Dockerfile Outdated Show resolved Hide resolved
entrypoint.sh Outdated Show resolved Hide resolved
This will allow users to more easily select the Terraform version
Signed-off-by: Dan Webb <dan.webb@damacus.io>
damacus added 2 commits July 21, 2022 08:28
- Stop using -l so that we get the environment loaded. This allows us to find the bundler binstubs
- Stop using bundle exec as we're now using binstubs.
- Install the Gems to a system location and install the binstubs,  so that we don't need a Gemfile to run Kitchen

Signed-off-by: Dan Webb <dan.webb@damacus.io>
Signed-off-by: Dan Webb <dan.webb@damacus.io>
Signed-off-by: Dan Webb <dan.webb@damacus.io>
@damacus damacus force-pushed the build-container branch 10 times, most recently from 18d1090 to 1168a30 Compare July 21, 2022 10:37
Signed-off-by: Dan Webb <dan.webb@damacus.io>
@damacus damacus requested a review from dan-hill2802 July 21, 2022 10:47
@damacus
Copy link
Contributor Author

damacus commented Jul 21, 2022

@dan-hill2802 this is ready to review again.

I'm going to see if I can tidy up anything before merging this afternoon 📦

damacus added 2 commits July 25, 2022 09:09
Signed-off-by: Dan Webb <dan.webb@damacus.io>
Signed-off-by: Dan Webb <dan.webb@damacus.io>
@damacus
Copy link
Contributor Author

damacus commented Jul 25, 2022

@dan-hill2802 this is good to go now. I've added the KICS scan to the new workflow so, I'm fairly sure we can drop the old workflow now.

Fix building first stage without arguments

Signed-off-by: Dan Webb <dan.webb@damacus.io>
.github/workflows/build.yaml Show resolved Hide resolved
.github/workflows/build.yaml Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
damacus added 2 commits July 25, 2022 11:57
Ignore apt version pinning

Signed-off-by: Dan Webb <dan.webb@damacus.io>
this support newer versions of Terraform

Signed-off-by: Dan Webb <dan.webb@damacus.io>
Signed-off-by: Dan Webb <dan.webb@damacus.io>
Copy link
Contributor

@dan-hill2802 dan-hill2802 left a comment

Choose a reason for hiding this comment

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

a few minor suggestions on the Actions

.github/workflows/build.yaml Outdated Show resolved Hide resolved
.github/workflows/build.yaml Show resolved Hide resolved
.github/workflows/build.yaml Outdated Show resolved Hide resolved
Signed-off-by: Dan Webb <dan.webb@damacus.io>
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@damacus
Copy link
Contributor Author

damacus commented Jul 27, 2022

@dan-hill2802 this workflow will fail until an internal contributor runs it because the workflow doesn't have the correct permissions to create/push the artefact right now.

@damacus damacus requested a review from dan-hill2802 July 28, 2022 08:11
@dan-hill2802
Copy link
Contributor

@dan-hill2802 this workflow will fail until an internal contributor runs it because the workflow doesn't have the correct permissions to create/push the artefact right now.

I'm going to merge and raise a new PR to confirm the new workflow. Releases are not automatic on this repo and the Quay builds have been disabled.

@damacus
Copy link
Contributor Author

damacus commented Jul 28, 2022

These images will all push with -edge right now. And I think that'll be fine. We should do a follow-up PR for a workflow that's triggered by a release.

@dan-hill2802 dan-hill2802 merged commit a493476 into dwp:main Jul 29, 2022
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