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

Update Dockerfile with ruby:3.3.4-alpine3.20 #3

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

roslynwythe
Copy link
Member

fixes hackforla/ops#142

Changes

Copy link
Member

@fyliu fyliu left a comment

Choose a reason for hiding this comment

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

Thanks for updating the docker image!

  1. I was able to build and confirm that the new image works just as well as the old one as far as I can tell.

    # build the image with the existing <repo>/<name> but with a 
    # testing tag
    docker build -t hackforlaops/ghpages:testing .
  2. I can confirm that the ruby version and the github-pages gem version are both updated.

    docker run -it hackforlaops/ghpages:testing sh
    
    # inside the container
    # check ruby version
    ruby --version
    
    # output
    ruby 3.3.4 ...
    
    # check gem version
    gem list | grep github-pages
    
    # output
    github-pages (232)
    ...

@danielridgebot
Copy link
Contributor

danielridgebot commented Aug 21, 2024

@fyliu Thank you for approving this PR. I don’t have rights to merge it either as @DanielRidge or as @roslynwythe. So that might be a discussion to have with the ops team and with Bonnie. But let’s first discuss testing. In my fork https://github.com/roslynwythe/ghpages-docker I created a new branch build-upload-image-with-testing-tag that is based on the PR branch, in which I modified .github/workflows/build-and-push-to-docker-hub.yml to upload the docker image with a distinct tag hackforlaops/ghpages:testing in place of hackforlaops/ghpages:latest

But to run that I would need the secrets with the dockerhub credentials. Or should I create a new dockerhub account for this purpose? Please confirm the advice of the ops team re: testing

@fyliu
Copy link
Member

fyliu commented Aug 21, 2024

@roslynwythe

I don't remember how, but I actually have write permission on this repo and can merge it. How would you like to merge? My own preference is "rebase and merge" to keep a linear history. No need to squash since your changes are clean and small already.

I recommend creating a personal docker hub account for testing. I didn't until I had to work on one of these docker image projects. Here's some random docker hub notes I made while working on a similar project in case they're useful. The token and password are interchangeable in the cli when it prompts for password, but for gha I think it's more secure to store the token as a secret since that can be revoked if it's ever leaked.

I'm not actually part of the ops team so I'm just speaking for myself.

@danielridgebot
Copy link
Contributor

danielridgebot commented Aug 22, 2024

Hi @fyliu. This is @roslynwythe again. Thank you for your help!

From my fork I modified the "build and push to docker hub" workflow and ran it so it uploaded to a new tag: hackforlaops/ghpages:testing to dockerhub

and we will let you know when testing of the new image is complete. It doesn't seem like the workflow will be triggered by merge, rather by push, so I'm curious how that will be done.

@roslynwythe
Copy link
Member Author

roslynwythe commented Aug 26, 2024

Hi @fyliu for testing the new image locally I have created a branch off gh-pages and updated docker-compose.yml to pull the image hackforlaops/ghpages:testing. See hackforla/website#7359. Please advise, for testing, should container_name remain with its current value hfla_site or should I define a new container name? In what other containers or scripts do we reference container_name?

@fyliu
Copy link
Member

fyliu commented Aug 26, 2024

Hi @roslynwythe. I don't think you need to create a website repo branch for testing, but you could. Just leave the container_name if it doesn't cause a conflict.

Generally, I think container_name is just unnecessary for projects using docker-compose. I did a quick search for "hfla_site" in the website project repository and didn't find any code or documentation that uses that container name. Looks like the project is only doing the normal thing of calling docker-compose up. The person who made the docker-compose.yml file probably followed a guide that told them to have it. But, it doesn't hurt anything, just not needed.

@roslynwythe
Copy link
Member Author

roslynwythe commented Aug 26, 2024

Thank you @fyliu for that information. I created a branch for testing and it looks like everything is working fine. But it appears that merging this PR will not trigger .github/workflows/build-and-push-to-docker-hub.yml so how does the team actually initiate that workflow?

@fyliu
Copy link
Member

fyliu commented Sep 6, 2024

@roslynwythe I think merging this PR should create a push event to trigger .github/workflows/build-and-push-to-docker-hub.yml.
I can test it in my fork which pushes to my personal docker hub repo.

Were you working on another workflow file to push a testing image on PR approval? I remember looking up how to do that but

helpful commands

I didn't find these commands in the wiki documentation but I want to add them somewhere

For images:

# build testing image
docker build . -t hackforlaops/ghpages:testing
# shell into a new container from image and remove container on exit
docker run --rm -it hackforlaops/ghpages:testing sh

For alpine package management (in an alpine container):

# update package information
apk update

# installed package version
apk list -I openssl
# available package version
apk info openssl

# install/upgrade package
apk add openssl
# remove package
apk del openssl

openssl vulnerability

From Docker Desktop, I also see an openssl vulnerability that was fixed 2 days ago. I'm not sure if that's something to be fixed in this issue. To update the version, add openssl to the list here

ghpages-docker/Dockerfile

Lines 96 to 101 in 9c91768

# Install bash and su-exec. These are required by the shell scripts we copied over right after starting build stage 2.
RUN apk --no-cache add \
bash \
su-exec

or to remove it: apk del openssl around the same place in the Dockerfile.

I'm not sure if the website will still work okay without it since it's not copied over from the build stage. The build stage installs libressl, which something might be using or not. libressl adoption seems to have gone down since 2020, so there might not be something that requires it anymore. I removed libressl and the openssl from the base image in all build stages and the build went through and runs fine locally. But I'm not using ssl.

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.

GHPAGES-DOCKER needs to be updated - ruby gem
3 participants