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

ci: overhaul and release automation #141

Conversation

k3rnelpan1c-dev
Copy link
Contributor

@k3rnelpan1c-dev k3rnelpan1c-dev commented Apr 25, 2022

Description

In the same veins as DependencyTrack/dependency-track#1419, this PR offers a CI overhaul and migrates the current release workflow from a bash script based solution to a GHA Workflow.

I took the liberty to include a proposal configuration for renovate-bot, as I personally find that a better configurable dependabot alternative for frontend repositories. An example of what this config would produce

Changes

  • update current CI workflows
  • overhaul the current Containerfile
  • add a Debian flavoured Containerfile
  • add a release workflow (can handle pre- and regular rerelease)
  • propose a renovate-bot config as it supports grouping and very granular configuration (opted to move to followup PR)

Issues

Signed-off-by: K3rnelPan1c <69395733+k3rnelpan1c-dev@users.noreply.github.com>
Signed-off-by: K3rnelPan1c <69395733+k3rnelpan1c-dev@users.noreply.github.com>
Signed-off-by: K3rnelPan1c <69395733+k3rnelpan1c-dev@users.noreply.github.com>
Signed-off-by: K3rnelPan1c <69395733+k3rnelpan1c-dev@users.noreply.github.com>
Signed-off-by: K3rnelPan1c <69395733+k3rnelpan1c-dev@users.noreply.github.com>
Signed-off-by: K3rnelPan1c <69395733+k3rnelpan1c-dev@users.noreply.github.com>
Signed-off-by: K3rnelPan1c <69395733+k3rnelpan1c-dev@users.noreply.github.com>
Copy link
Member

@nscuro nscuro left a comment

Choose a reason for hiding this comment

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

Overall this looks solid, just a few minor comments that may need clarification.

Also huge +1 for using Renovate, it's much better than Dependabot. It will need to be activated first however.

.github/renovate.json Outdated Show resolved Hide resolved
.github/workflows/ci-release.yaml Outdated Show resolved Hide resolved
docker/Dockerfile.alpine Outdated Show resolved Hide resolved
docker/Dockerfile.alpine Outdated Show resolved Hide resolved
docker/Dockerfile.debian Outdated Show resolved Hide resolved
docker/Dockerfile.debian Outdated Show resolved Hide resolved
docker/docker-entrypoint.sh Outdated Show resolved Hide resolved
.github/renovate.json Outdated Show resolved Hide resolved
.github/renovate.json Outdated Show resolved Hide resolved
Signed-off-by: K3rnelPan1c <69395733+k3rnelpan1c-dev@users.noreply.github.com>
@k3rnelpan1c-dev k3rnelpan1c-dev force-pushed the ci/overhaul-and-release-automation branch from 7a4690e to 2a61e12 Compare April 29, 2022 15:40
@nscuro
Copy link
Member

nscuro commented May 10, 2022

@k3rnelpan1c-dev @stevespringett I think we should decide on either Debian or Alpine. There's really no reason to have both. I'd personally vouch for Alpine - smaller attack surface and I've never seen issues with it. See also @k3rnelpan1c-dev's response here: #78 (comment)

I'd rather not have the complexity of handling multiple flavors in CI if we can avoid it.

Thoughts?

@stevespringett
Copy link
Member

The API Server moved to Debian due to use of glibc and to avoid some issues that Alpine has with K8s.

The frontend we have a bit more flexibility over. I also prefer Alpine due to the smaller attack surface, but I need to find out what that K8s issue with Alpine was. But I would prefer to have a single base image rather than multiple.

@k3rnelpan1c-dev
Copy link
Contributor Author

I cannot imagine Alpine having any issue on K8s, after all the most commonly used external Ingress controller happens to be Nginx, which is using its Alpine based version IIRC.

But I am curious, if you happen to find the reference / remember what this issue might have been that would be great input. The only "issue" that I am aware with Alpine was that programs relying on glibc could experience very odd behaviour, however that was in every container environment not just K8s.

@nscuro
Copy link
Member

nscuro commented May 10, 2022

Known issues appear to evolve around DNS resolution. Small selection:

The K8s team themselves claim:

If you are using Alpine version 3.3 or earlier as your base image, DNS may not work properly due to a known issue with Alpine. Kubernetes issue 30215 details more information on this.

The frontend doesn't do any DNS resolution though. Even if the issue(s) above persist, it won't be a problem for this particular container.

Signed-off-by: K3rnelPan1c <69395733+k3rnelpan1c-dev@users.noreply.github.com>
@k3rnelpan1c-dev
Copy link
Contributor Author

k3rnelpan1c-dev commented May 10, 2022

🤦‍♂️ I actually heard of that before. Thank you for the detailed response!

Side-note: fun that it is used for an ingress then when using the Nginx Ingress xD

Edit:
I second that, even should this bug still exists today in Alpine Linux 3.15+, that the frontend would continue to work as all it does is host the static HTML and JS via an ingress, reveres proxy or directly to to a end user. Thereby not needing any DNS resolution at all.

Signed-off-by: K3rnelPan1c <69395733+k3rnelpan1c-dev@users.noreply.github.com>
@k3rnelpan1c-dev
Copy link
Contributor Author

k3rnelpan1c-dev commented May 12, 2022

took the liberty and removed the Debian flavour from the build and release process, as we all seem to be in agreement that we would prefer to only offer one flavour and that the minimal attack surface of Alpine Linux is favourable.

@nscuro nscuro force-pushed the ci/overhaul-and-release-automation branch from 9ce99b3 to f13aef4 Compare May 12, 2022 21:31
@nscuro
Copy link
Member

nscuro commented May 12, 2022

@k3rnelpan1c-dev Apologies for my commits. Wanted to test in my fork but forgot to specify the correct remote when pushing my changes... May I ask you to hard-reset them, verify that everything's still as you intended and force push? 🥲

@nscuro
Copy link
Member

nscuro commented May 12, 2022

Anyway, I was now able to test this and it works like a charm!

Once we have my mess 🥲 cleaned up it's good to go from my side.

Signed-off-by: K3rnelPan1c <69395733+k3rnelpan1c-dev@users.noreply.github.com>
@k3rnelpan1c-dev k3rnelpan1c-dev force-pushed the ci/overhaul-and-release-automation branch from f13aef4 to 37cec71 Compare May 12, 2022 22:53
@k3rnelpan1c-dev
Copy link
Contributor Author

k3rnelpan1c-dev commented May 12, 2022

all done + added a tiny hotfix that we should consider adding to the backed CI eventually.
The login against docker.io should only really run when we are actually attempting to push the image to it (or any registry for that matter).

edit:
applied the hotfix with another oversight fix to the backbend CI 😃
DependencyTrack/dependency-track#1623

@k3rnelpan1c-dev
Copy link
Contributor Author

k3rnelpan1c-dev commented May 12, 2022

Thank you for testing this nscuro :)
+ I could have locked the PR if I wanted to prevent upstream maintainers to change my changes, so no harm done in any way.

Copy link
Member

@nscuro nscuro left a comment

Choose a reason for hiding this comment

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

All good from my side, just need @stevespringett to approve it before we can merge.

@stevespringett stevespringett merged commit 4db104c into DependencyTrack:master May 17, 2022
@stevespringett
Copy link
Member

Huge thanks @k3rnelpan1c-dev

@k3rnelpan1c-dev
Copy link
Contributor Author

k3rnelpan1c-dev commented May 17, 2022

I enjoy working on CI and automation, happy to have helped out :)
thank you for the great project!

@k3rnelpan1c-dev k3rnelpan1c-dev deleted the ci/overhaul-and-release-automation branch May 17, 2022 17:59
@nscuro
Copy link
Member

nscuro commented May 18, 2022

It looks like, when releasing, _meta-build.yml/build-node does not "see" the version bump from ci-release.yml/prepare-release. actions/checkout per default checks out the Git commit that triggered the workflow run, which will always be the one before npm version was called in ci-release.yml/prepare-release.

For example, https://github.com/DependencyTrack/frontend/runs/6483307556?check_suite_focus=true checked out 01b253c instead of 3fe6bf7.

This results in the version field of package.json not being correct, which also shows in the UI:
image

I'm wondering what would be the best strategy to resolve this. Some ideas:

  • Decouple prepare-release from the rest of the workflow. Essentially, only run npm version (which creates a Git tag)
    • Introduce another workflow that's triggered by new tags. Run builds and create-release in that workflow
  • Somehow make _meta-build.yml aware that the to-be-built commit changed. I feel like this may be wonky and error prone.

Would love to get your input @k3rnelpan1c-dev, your expertise is greatly appreciated.

@k3rnelpan1c-dev
Copy link
Contributor Author

k3rnelpan1c-dev commented May 19, 2022

Oha, well this is interesting and certainly unexpected behaviour 🤔
I will try and toy around with this the coming weekend and see whats up with this behaviour.

Anyway, for feedback on your workaround proposals: Both can work.
We could split off the build and publish part of the release CI and have it only create a GH Release that would trigger a build + publish CI or, give I can figure out why GHA behaves the way it does, we fix whatever causes it. (given we can, as it dose not seem to have that problem in the backend CI)

EDIT:
I too am not a big fan of somehow passing the ref to check out to the "_meta" workflow.

@k3rnelpan1c-dev
Copy link
Contributor Author

Okay I thought of this a bit more and it actually makes sense that by default every checkout step within the same workflow checks out the state that it got triggered with .... (and now I am annoyed that I didn't think of this earlier)

In other news I have a feeling that the idea of creating a pure "create release" workflow and letting a "publish" workflow trigger on "release creation" might be the right way to move forward.

I will analyse more, but otherwise would commit to that idea and iterate over the CI that way.
Given you are on board with that idea, ofc.

@k3rnelpan1c-dev
Copy link
Contributor Author

Okay, tiny problem with the separated workflows: No matter how I change it using the default GITHUB_TOKEN cannot be used to realise it ☹️

Note on GitHub Actions: You can use the default token which is provided in the secret GITHUB_TOKEN. However releases done with this token will NOT trigger release events to start other workflows.
If you have actions that trigger on newly created releases, please use a generated token for that and store it in your repository's secrets (any other name than GITHUB_TOKEN is fine).

So to say that I am stuck with that idea is quite accurate, unless we opt to setup a "Dependency-Track-Bot" account that will serve as PAT donor to create the release and commits to the projects on release.

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.

Migrate Docker image from Alpine to Debian Slim
3 participants