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

feat: git push support #21

Merged
merged 22 commits into from
Feb 7, 2024
Merged

feat: git push support #21

merged 22 commits into from
Feb 7, 2024

Conversation

neurosnap
Copy link
Member

@neurosnap neurosnap commented Jan 26, 2024

FEAT

jobs:
  deploy:
    runs-on: ubuntu-latest
    steps:
    - name: Deploy to Aptible
      uses: aptible/aptible-deploy-action@v2
      with:
        type: git
        app: <app name>
        environment: <environment name>
        username: ${{ secrets.APTIBLE_USERNAME }}
        password: ${{ secrets.APTIBLE_PASSWORD }}

BREAKING CHANGES

  • type is now required (choices: git or docker)

Approach

We tried to make this as congruent as possible with the current Direct Docker Image Deploy strategy. In that effort, we wanted to support git push without requiring yet another authentication secret -- the SSH private key -- so we had to jump through some hoops to use our Aptible access token for pushing to our git remote.

Security Considerations

We are interfacing directly with primetime in order to perform a git push using just an Aptible token. This makes this paradigm more visible to the outside world, something to think about.

entrypoint.sh Outdated
REMOTE_URL="root@$INPUT_GIT_REMOTE:$INPUT_ENVIRONMENT/$INPUT_APP.git"
git remote add aptible ${REMOTE_URL}
BRANCH="deploy-$(date "+%s")"
GIT_SSH_COMMAND="ssh -o SendEnv=ACCESS_TOKEN -o PubkeyAuthentication=no -o StrictHostKeyChecking=no -p 43022" git push aptible "main:$BRANCH"
Copy link
Member Author

Choose a reason for hiding this comment

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

"main:$BRANCH" needs to change to whatever branch is currently checked out.

Comment on lines +62 to +64
To use this image, you should use another workflow step to publish your image to
a Docker image registry (for example
[Docker's](https://github.com/marketplace/actions/build-and-push-docker-images)).
Copy link
Member

Choose a reason for hiding this comment

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

Future Improvement: It may be useful to provide an example of how to build and push to the repo's GitHub packages registry since my understanding is it should "just work" without any external setup.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think you are right, but can we look into this after this is deployed?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, that's why I marked it as "Future Improvement".

README.md Outdated Show resolved Hide resolved
entrypoint.sh Show resolved Hide resolved
@neurosnap neurosnap marked this pull request as ready for review February 2, 2024 16:42
@neurosnap neurosnap requested a review from aguilinger as a code owner February 2, 2024 16:42
@neurosnap neurosnap requested a review from joshraker February 2, 2024 16:42
Copy link
Member

@aguilinger aguilinger left a comment

Choose a reason for hiding this comment

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

Looks good to me, just one nit but not critical. I'll leave Josh to give the 'Approve' but has my 👍

action.yml Outdated
git_remote:
description: 'Aptible git remote domain'
required: False
default: elb-aptible-us-east-1-81550.aptible.in
Copy link
Member

Choose a reason for hiding this comment

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

Why use the internal domain and not either git.aptible.com or beta.aptible.com since those are much more likely to stay static?

Copy link
Member

Choose a reason for hiding this comment

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

That's my bad. Eric asked me what the hostname for primetime was and I didn't see a user domain on it so I just gave him the internal hostname. I overlooked the possibility of using a custom cert which would cause the endpoint to not have user domain. We should use git.aptible.com.

Copy link
Member

@UserNotFound UserNotFound Feb 2, 2024

Choose a reason for hiding this comment

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

We should use primetime.aptible.com, since it's a different service than Megatron (git.aptible.com).

Copy link
Member

Choose a reason for hiding this comment

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

Or no certificate because it's using ssh not http 🤦

Copy link
Member

Choose a reason for hiding this comment

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

primetime.aptible.com ?

Is that what we should use for a git push?

Copy link
Member

Choose a reason for hiding this comment

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

We should use primetime.aptible.com, since it's a different service than Megatron (git.aptible.com).

Is that the right service to push to? Normally a user would push to beta.aptible.com or git.aptible.com which both point to megatron, right?

Copy link
Member

Choose a reason for hiding this comment

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

We are interfacing directly with primetime in order to perform a git push using just an Aptible token. This makes this paradigm more visible to the outside world, something to think about.

Alex is right I glossed over that

entrypoint.sh Outdated
REMOTE_URL="root@$INPUT_GIT_REMOTE:$INPUT_ENVIRONMENT/$INPUT_APP.git"
git remote add aptible ${REMOTE_URL}
REMOTE_BRANCH="deploy-$(date "+%s")"
GIT_SSH_COMMAND="ssh -o SendEnv=ACCESS_TOKEN -o PubkeyAuthentication=no -o StrictHostKeyChecking=no -p 43022" git push aptible "$GITHUB_BRANCH:$REMOTE_BRANCH"
Copy link
Member

Choose a reason for hiding this comment

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

StrictHostKeyChecking=no
We should not skip hostkey checking from a location outside of our control, eg GHA runners. (this will necessitate persisting it between deployments of Primetime with a PersistentDisk)

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated. Will have to re-test this to make sure it all works still

Copy link
Member

@joe-herman joe-herman left a comment

Choose a reason for hiding this comment

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

@aguilinger
Copy link
Member

We might add an extra note about 'Breaking changes' in the release notes and call out that existing users need to add the 'type: docker' option

@neurosnap neurosnap merged commit d6d2401 into master Feb 7, 2024
@neurosnap neurosnap deleted the git-push-support branch February 7, 2024 20:35
fancyremarker pushed a commit that referenced this pull request Feb 7, 2024
This reverts commit d6d2401.
This reverts commit 7feeacd.
fancyremarker added a commit that referenced this pull request Feb 7, 2024
This reverts commit d6d2401.
This reverts commit 7feeacd.

Co-authored-by: Frank Macreery <frank@macreery.com>
neurosnap added a commit that referenced this pull request Feb 8, 2024
BREAKING CHANGE: `type` is now required (choices: `git` or `docker`)
@neurosnap neurosnap mentioned this pull request Feb 15, 2024
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.

5 participants