GitHub Action for building docker images #44
Conversation
derekclair
left a comment
derekclair
left a comment
There was a problem hiding this comment.
While incomplete for all use-cases, this, as a "template", for building the docker images does work and may help address a variety of related issues/concerns.
| ${{ env.REGISTRY }}/platform-appserver | ||
| tags: | | ||
| type=raw,value={{branch}}-{{sha}} | ||
| type=sha,prefix={{branch}}-,enable={{!is_default_branch}} |
There was a problem hiding this comment.
I'm not entirely sure that this enable={{!is_default_branch}} syntax will work, it's un-verified but, none the less it expresses the intent. I'll work to verify this before opening the PR.
There was a problem hiding this comment.
If we have not adopted github enterprise should we get this working in Jenkins first or also?
There was a problem hiding this comment.
Great point @LucasThelen! Glad you mentioned it.
Is this the fish you wish? #45
| # secret-files: | | ||
| # oidc=${{ secrets.oidc_config }} |
There was a problem hiding this comment.
| # secret-files: | | |
| # oidc=${{ secrets.oidc_config }} | |
| secret-files: | | |
| oidc=${{ secrets.oidc_config }} |
This, along with the included changes recommended to the dockerfile.appserver would facilitate a secure method for providing the OIDC configuration to the build. (An exclusively GitHub Actions flavored method, but it would serve as a guide regardless.)
| COPY --chown=www-data:www-data appserver_config/ /opt/iqgeo/config/ | ||
| # RUN --mount=type=secret,id=oidc,target=/opt/iqgeo/config/oidc/conf.json |
There was a problem hiding this comment.
| COPY --chown=www-data:www-data appserver_config/ /opt/iqgeo/config/ | |
| # RUN --mount=type=secret,id=oidc,target=/opt/iqgeo/config/oidc/conf.json | |
| RUN --mount=type=secret,id=oidc,target=/opt/iqgeo/config/oidc/conf.json |
This will need some re-touching if it is to account for all 'appserver_config' scenarios, but does illustrate exposing secrets to the build as a best practice.
There was a problem hiding this comment.
Do you have to add this to the tools image as well?
There was a problem hiding this comment.
@LucasThelen Negative. At least, not in current implementation. It does not COPY anything from the host file system.
Now, that said, if tools image does need the /opt/iqgeo/config/ directory (meaning, it does use the OIDC configuration currently), its unclear to me how that directory/path is getting into that image now, because it is not explicitly doing so in the dockerfile.tools.
Obviously, if that should be a requirement, yes, it can be done there too.
for multi-arch builds
| runs-on: ubuntu-latest | ||
| env: | ||
| REGISTRY: harbor.delivery.iqgeo.cloud/${{ vars.harbor_project_name }} | ||
| platform_version: 7.2 |
There was a problem hiding this comment.
dont we want version to be a variable? that would allow you to have one action that can build multiple images.
There was a problem hiding this comment.
I considered how that might go, yes (and spent way more time than I'd have liked to). But, in the end, the problem as I see it is; what is the 'source of truth' for that? The GitHub repository? It could work for the workflow, yes, but what about dev environments...? The .iqgeorc file? Yes, probably. Or, at least, that is the status-quo. While the source of truth it's NOT the Action's workflow, it's parameterized here for convenience and ease-of-use for when the time comes that it does need to be changed.
Or, that's my $0.02 on the matter. RFC!
| ${{ env.REGISTRY }}/platform-build:${{ env.platform_version }} | ||
| iqgeo-myproj-build |
There was a problem hiding this comment.
@luiscamachopt @mstrong98 @tomthetommy @ianrobertson-iqgeo so the record shows it, this is NOT a good tagging strategy.
I see this, adding a CI/CD pipeline definition to this project template, as a golden opportunity for us to establish a tagging strategy; let's be sure to capitalize on it, and let's work toward consensus on what our tagging strategy is.
If ever we wanted to get tagging right, here is our chance!
There was a problem hiding this comment.
I'm considering taking another pass at this @luiscamachopt to reflect my most recent (and most current iteration of this workflow) which, personally, I feel to be my best iteration to-date. But, I don't want to jump the gun on the conversation here. Will you @luiscamachopt take a look at this implementation too? https://github.com/IQGeo/project-nmti-trials/blob/dba9e106e092009df2678083b66368de944dad89/.github/workflows/sre-image-build.yml
|
@derekclair you'll need to rebase this. it's a bit out of date, the dockerfiles now have a distinction between PRODUCT_REGISTRY and PROJECT_REGISTRY. I suggest rebasing onto the current state of the |
Proposed solution to addresses a host of related issues. (Including SRE-187)