Skip to content
This repository was archived by the owner on Feb 27, 2024. It is now read-only.

🚀 Webapp docker registry push using composite actions#2

Open
rahulpatidar0191 wants to merge 52 commits intomainfrom
feature/webapp-deployment-shell
Open

🚀 Webapp docker registry push using composite actions#2
rahulpatidar0191 wants to merge 52 commits intomainfrom
feature/webapp-deployment-shell

Conversation

@rahulpatidar0191
Copy link
Copy Markdown
Member

@rahulpatidar0191 rahulpatidar0191 commented Nov 2, 2022

This action builds and pushes an image to docker registry with correct format for portainer

Closes: https://github.com/SatelCreative/DevOps-IT/issues/205

Tested on

  1. SB-PIM : Actions
  2. ST-PIM : Actions

@rahulpatidar0191 rahulpatidar0191 added the Type: Enhancement New feature or request label Nov 2, 2022
@rahulpatidar0191 rahulpatidar0191 self-assigned this Nov 2, 2022
@rahulpatidar0191 rahulpatidar0191 removed the request for review from hillairet November 4, 2022 00:13
TAG_NAME=${11}


echo "APP_NAME=${APP_NAME}, WORK_DIR=${WORK_DIR}, SATEL_DOCKER_USER=${SATEL_DOCKER_USER}, SATEL_DOCKER_PASS=${SATEL_DOCKER_PASS}, CLIENT_DOCKER_USER=${CLIENT_DOCKER_USER}"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Don't print out passwords into logs.

Suggested change
echo "APP_NAME=${APP_NAME}, WORK_DIR=${WORK_DIR}, SATEL_DOCKER_USER=${SATEL_DOCKER_USER}, SATEL_DOCKER_PASS=${SATEL_DOCKER_PASS}, CLIENT_DOCKER_USER=${CLIENT_DOCKER_USER}"
echo "APP_NAME=${APP_NAME}, WORK_DIR=${WORK_DIR}, SATEL_DOCKER_USER=${SATEL_DOCKER_USER}, CLIENT_DOCKER_USER=${CLIENT_DOCKER_USER}"

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Nope, I was using that to debug, but will remove them, it prints them as **** as well

README.md Outdated
Comment on lines +31 to +36
satel-docker-user: ${{ secrets.SATEL_DOCKER_USER }}
satel-docker-pass: ${{ secrets.SATEL_DOCKER_PASS }}
client-docker-user: ${{ secrets.CLIENT_DOCKER_USER }}
client-docker-pass: ${{ secrets.CLIENT_DOCKER_PASS }}
satel-registry: docker.satel.ca
client-registry: sb-docker.satel.ca
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ok here is a place to simplify and also create more modularity in a way:
Don't define "satel" and "client" registries. Instead just have one docker registry defined and have the user handle the switch between the registries.
Typically we'll push the latest commit on main to the Satel registry for QA and a release/tag to the client's registry for staging and production, so the user can or even will have to setup 2 workflows. They can switch the docker registry.
This way this action is simpler, doesn't enforce/assume anything, and then the workflow is more modular/flexible.
Is there maybe even an action that already does what we need here if it's super general?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

hmm, I pass them both but only push to client's if it's a tag .
I am not sure how we will get the user to do that switch. I'll into it.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You did two different workflow for tag and main for JD's project ... Why not the same for that?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

True, I think this is what you had in mind when you said we can probably replace this action completely now
https://github.com/marketplace/actions/docker-build-push-action

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I guess we'd rely on the QA environment to see if the docker image is working properly.
It's fine if we get notified of failures right away on the main branch jobs.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We only use this action to build and push to registry, the testing of the docker image is done via this action

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ah brilliant. Then we should really switch to a generic public one instead of reinventing the while!
The one thing is that we need the Dockerfile to be production only in that case ...
We really to reorganize it all.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Type: Enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants