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

Cleanup ci and add trivy #1239

Merged
merged 15 commits into from
Jun 14, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 0 additions & 15 deletions .github/workflows/assert-signed-commits.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

Why have we removed this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't I just moved it into CI

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah it's in the CI job. I think that's a bad idea because of the pull_request_target keyword

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why do you think it's a bad idea?

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How would moving it back protect us?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@ale8k ale8k Jun 14, 2024

Choose a reason for hiding this comment

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

Idk if I'm understanding correctly, but if they made a pr regardless and it wanted to get secrets, doesn't seem to matter where the job is, it's just the fact you're using that trigger? So our solution is to remove the use of it?

Copy link
Contributor

Choose a reason for hiding this comment

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

From the SO post,

This event is given repo secrets and a full read/write GITHUB_TOKEN to boot, however there is a catch - this action only runs in the pull request's target branch, and not the pull request's branch itself.

I believe this means that the CI job will not check the code from your fork, so the test will pass not because your code is necessarily correct but because the test will check out code from e.g. v3 JIMM.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not really sure what you mean? Let's move this to MM.

This file was deleted.

57 changes: 41 additions & 16 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
@@ -1,31 +1,55 @@
name: CI
on:
pull_request:
pull_request_target:
workflow_call:
workflow_dispatch:


jobs:
# lint:
# runs-on: ubuntu-20.04
# continue-on-error: true
# steps:
# - uses: actions/checkout@v3
# - uses: actions/setup-go@v4
# with:
# go-version-file: 'go.mod'
# - name: golangci-lint
# uses: golangci/golangci-lint-action@v3
# with:
# # Optional: version of golangci-lint to use in form of v1.2 or v1.2.3 or `latest` to use the latest version
# version: latest
# skip-pkg-cache: true
# skip-build-cache: true
check-signed-commits:
name: Check signed commits in PR
runs-on: ubuntu-latest
permissions:
contents: read
pull-requests: write
steps:
- name: Check signed commits in PR
uses: 1Password/check-signed-commits-action@v1

security_scan:
name: Security Scan
runs-on: ubuntu-22.04
timeout-minutes: 45
steps:
- uses: actions/checkout@v4
Copy link
Contributor

Choose a reason for hiding this comment

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

This one is missing a name

Copy link
Contributor Author

Choose a reason for hiding this comment

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

LOL

with:
fetch-tags: true
fetch-depth: 0

- name: Run govulncheck
uses: golang/govulncheck-action@v1

- name: Run Trivy vulnerability scanner in repo mode
uses: aquasecurity/[email protected]
with:
scan-type: 'fs'
ignore-unfixed: true
format: 'table'
# output: 'trivy-results.sarif' # TODO(ale8k) Turn on when uploading to gh and change above line to sarif
severity: 'CRITICAL'
exit-code: '1'

# TODO(ale8k): Setup GH security
# - name: Upload Trivy scan results to GitHub Security tab
# uses: github/codeql-action/upload-sarif@v2
# with:
# sarif_file: 'trivy-results.sarif'

build_test:
name: Build and Test
runs-on: ubuntu-22.04
timeout-minutes: 45
needs: [security_scan]
steps:
- uses: actions/checkout@v4
with:
Expand Down Expand Up @@ -60,6 +84,7 @@ jobs:
smoke_test:
name: Smoke Test
runs-on: ubuntu-22.04
needs: [security_scan]
# The docker compose has a healthcheck on the JIMM container.
# So if the compose returns with exit code 0 then the JIMM server successfully started.
steps:
Expand Down
15 changes: 0 additions & 15 deletions .github/workflows/jaas-snap-release.yaml

This file was deleted.

16 changes: 0 additions & 16 deletions .github/workflows/jimmctl-snap-release.yaml

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
# Publish the OCI image to ghcr
name: Publish server image
name: Release Server ROCK

on:
# Note that when running via workflow_dispatch, the github.ref_name
Expand Down
31 changes: 31 additions & 0 deletions .github/workflows/release-snaps.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
name: Release Snaps

on:
workflow_dispatch:
push:
tags:
- 'v3*'

jobs:
build-and-release-jimmctl:
uses: ./.github/workflows/snap-release.yaml
ale8k marked this conversation as resolved.
Show resolved Hide resolved
with:
folder: jimmctl
release-channel: 3/edge
secrets: inherit

build-and-release-jaas-plugin:
uses: ./.github/workflows/snap-release.yaml
ale8k marked this conversation as resolved.
Show resolved Hide resolved
with:
folder: jaas
release-channel: 3/edge
secrets: inherit

build-and-release-jimm-server:
uses: ./.github/workflows/snap-release.yaml
Copy link
Collaborator

Choose a reason for hiding this comment

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

missing name

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a composite workflow, cannot be named

with:
jobs: build # Only build, this snap isn't released to snapcraft
folder: jimm
release-channel: 3/edge # Not used for this snap
secrets: inherit

4 changes: 3 additions & 1 deletion .github/workflows/snap-release.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be moved to the actions folder like you suggested?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no no, i was gonna break it into one actions but not really any point after looking at them, think composite workflow is fine .. if we break into one action we can't use "jobs" param to run up to a point as you know, it's just a single action...

Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
name: Release snap
# Release Snap is a composite workflow used within other workflows
# to reuse the logic of building and publishging a snap in one.
name: Release Snap

on:
workflow_call:
Expand Down
29 changes: 0 additions & 29 deletions .github/workflows/snap.yaml

This file was deleted.

Loading