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

Add "delete" command to the CLI #167

Open
queicherius opened this issue Dec 15, 2020 · 9 comments
Open

Add "delete" command to the CLI #167

queicherius opened this issue Dec 15, 2020 · 9 comments
Labels
feature New feature
Projects

Comments

@queicherius
Copy link

I would like to see a "delete" command added to the Meli CLI that deletes the deploy of a specific branch. In my use-case, which is deploying branches via CI as previews, each PR generates a deploy, and I would like to be able to automatically clean it up once the PR gets closed. Right now I have to head over into the UI to click the delete button.

@gempain
Copy link
Contributor

gempain commented Dec 15, 2020

Aww, yup, that's a good one too. Just quickly brainstorming before going deeper, how would we detect that the PR has been closed ? When the PR is merged, a CI pipeline runs for the base branch, and in there I don't think we can tell which PR has been merged, do we ? GH Actions sets GITHUB_HEAD_REF and GITHUB_BASE_REF as an environment variable, but I don't think their available once running on the base branch.

We could add a --delete-branch CLI option which would let you specify the branches to delete, but it feels bit unsafe, no ? I mean, you still have to get access to the token, which indicates you have access to the site, but that leak could come from other places.

The cleanest approach I can think of is to setup webhooks on your Git server to let Meli know when a PR is closed, but I don't want to create this dependency at the moment, it's too early and there's a bunch of test coverage to increase before adding extra dependencies.

Do you have an idea of how we would implement this ?

@queicherius
Copy link
Author

queicherius commented Dec 15, 2020

I was thinking of having a separate workflow, similar to this (untested, pseudo-code)

name: Continuous Integration

on:
  pull_request:
    types: [closed]

jobs:
  cleanup:
    name: 'Cleanup'
    runs-on: ubuntu-latest

    steps:
      - name: 'Cleanup deploy (documentation)'
        run: |
          npx @getmeli/cli@next delete \
            --url "https://meli.company.io" \
            --site "$MELI_SITE" \
            --token "$MELI_TOKEN" \
            --branch "$GITHUB_BRANCH"
        env:
          MELI_SITE: '<site uuid>'
          MELI_TOKEN: ${{ secrets.MELI_TOKEN }}
          GITHUB_BRANCH: ${{ github.payload.pull_request.head.ref }}

@pimartin
Copy link
Contributor

I'd even go further and start building a structure of subcommands, a bit like the docker-cli:

  • meli site
    • meli site branch
      • meli site branch create
      • meli site branch list
      • meli site branch remove
    • meli site release
      • meli site release create
      • meli site release list
      • meli site release remove
  • ...
    (there is no need to have all commands immediately, but we can start preparing for the future)

In the end, the CLI could allow users to manipulate most resources (if not all). This would be ideal for automation (CI, scripts, Ansible...).

@gempain
Copy link
Contributor

gempain commented Dec 15, 2020

@queicherius interesting, I haven't used GH Actions enough to know this, but it's perfect for the use case you provided, and keeps us from creating a dependency. I love how you creating you guys can be.

I'm in for the meli site branch delete <branch> and the usual --url ... options. What do you think ?

@queicherius
Copy link
Author

I'd happily use that command :) Maybe it'd be a good idea to write out all possible commands up front to create a consistent CLI experience - the command you proposed would work quite different than the current upload one (where branch is required, but provided via a named rather than a positional argument)

@gempain
Copy link
Contributor

gempain commented Dec 15, 2020

Plus I don't know if you've experienced this issue, but when running the CLI in multiline mode, I sometimes get errors when --branch my-branch \ is the last line before the path. I'm not crazy, we've seen this several times but I can't always reproduce.

@gempain gempain added the feature New feature label Dec 15, 2020
@gempain
Copy link
Contributor

gempain commented Dec 16, 2020

@queicherius I'll prioritize this feature for the next release as I think this is very important. Not very active today as I was fixing the search on our docs which has been broken for 4 days now as well as doing a bunch of small other stuff 🚀

@gempain
Copy link
Contributor

gempain commented Dec 17, 2020

So I just had a though on this. @queicherius there is a workaround until we implement this feature. You could use the API ! It's a bit tedious but if you really need it urgently this is something that can work. Here's how to do it:

  1. Get an API token from your user settings, and only toggle scopes site.branch.list and site.branch.delete (I like this a lot BTW, being able to discriminate token scopes on specific endpoints).
  2. Get the site via the API
  3. Find the branch ID
  4. Delete the branch via the API

It's doable with CURL but I'm not a bash expert, so here's in JS (I tested this and it works like a charm):

require('dotenv/config');

const url = 'https://meli.site.com';
const siteId = '<your-site-id>';

const axios = require('axios').create({
  baseURL: url,
  headers: {
    'X-Token': process.env.API_TOKEN,
  },
});

async function deleteBranch(name) {
  const { data } = await axios.get(`/api/v1/sites/${siteId}/branches`);

  const branch = data.find(branch => branch.name === name);

  if (!branch) {
    console.log('Branch not found');
    return;
  }

  await axios.delete(`/api/v1/sites/${siteId}/branches/${branch._id}`);
}

deleteBranch('demo').catch(console.error);

One limitation to outline is that the token is not scoped to a specific site. This means technically someone could delete other branches in other sites, but you'd have to know the ID in advance. This wouldn't be an issue with the site token, but right now it can only be used to access the release upload endpoint. I gues this isn't a huge issue if you're the only collaborator in your project or if you trust your collaborators (which I hope you do anyway).

Some improvements we need to make to the API:

  • allow calling branch related endpoints with a URI encoded branch name instead of a branch ID, this would prevent having to do two calls to get the branch ID
  • prevent deleting the main branch from the API / CLI to avoid someone trying to down your site. This could be configurable at the site level.

Edit: I added some docs here

@queicherius
Copy link
Author

Awesome, thank you! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature
Projects
main
To do
Development

No branches or pull requests

3 participants