Add missing dependencies for the TICS action #1212
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1212 +/- ##
=======================================
Coverage ? 87.64%
=======================================
Files ? 91
Lines ? 6231
Branches ? 111
=======================================
Hits ? 5461
Misses ? 714
Partials ? 56 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
.github/workflows/tics-run.yaml
Outdated
| protoc-gen-go | ||
| protoc-gen-go-grpc |
There was a problem hiding this comment.
do those exist as debian packages? I thought we install those via go
There was a problem hiding this comment.
They do, but you're correct... it's probably better to install those using Go to make sure we have the right versions
We need to compile the proto files, which means we need the tools used for the compilation.
denisonbarbosa
force-pushed
the
fix-tics-action
branch
from
43377ea to
a26c1d0
Compare
| sudo apt-get install -y ${{ env.build_dependencies }} | ||
|
|
||
| go install honnef.co/go/tools/cmd/staticcheck@latest | ||
| go install google.golang.org/protobuf/cmd/protoc-gen-go@latest |
There was a problem hiding this comment.
I think we should run go install in the tools directory and without the trailing @latest, so that it will install the exact version that's configured in tools/go.mod, similar to how it's done in https://github.com/canonical/desktop-engineering/blob/0466f1f6e2ab7044a030731193af9ecabd2c7281/gh-actions/go/generate/action.yaml#L30-L47
There was a problem hiding this comment.
Good point. I think it's easier if we just use the action from the desktop-engineering repo then. I'll also update the commands to also build the brokers now that we have the code in the repo.
There was a problem hiding this comment.
makes sense. if we use the generate action, we also don't have to call go generate -C pam -x ourselves, right?
There was a problem hiding this comment.
I'll also update the commands to also build the brokers now that we have the code in the repo.
That's already done by 2e60734, you just have to rebase on main
There was a problem hiding this comment.
That's already done by 2e60734, you just have to rebase on main
Since we're touching the code here again, we could fix the name of the authd-oidc broker binary, in this file it's currently built as authd-vanilla
There was a problem hiding this comment.
That's already done by 2e60734, you just have to rebase on main
Nice! However, I feel like it might not be that simple... Since authd and authd-oidc-brokers were created as separate projects in the TICS dashboard, I think running the analysis when the code is under the authd project will affect authd's report (and I'm not even sure we can still get the oidc report). I was trying to do some investigation running it locally, but the tool keeps hanging. I'll probably have to contact them directly.
There was a problem hiding this comment.
Since authd and authd-oidc-brokers were created as separate projects in the TICS dashboard, I think running the analysis when the code is under the authd project will affect authd's report
now that they are not separate repos anymore, it only makes sense to have one single TiCS report which includes both authd and authd-oidc-brokers, no?
There was a problem hiding this comment.
Indeed, I agree. I'll reach out to them to ask for merging the "excluded files" list in a single project
| sudo apt-get install -y ${{ env.build_dependencies }} | ||
|
|
||
| go install honnef.co/go/tools/cmd/staticcheck@latest | ||
| go install google.golang.org/protobuf/cmd/protoc-gen-go@latest |
There was a problem hiding this comment.
Does this also install protoc-gen-go-grpc? I see that tools/go.mod only lists google.golang.org/grpc/cmd/protoc-gen-go-grpc while tools/tools.go has both google.golang.org/grpc/cmd/protoc-gen-go-grpc and github.com/golang/protobuf/protoc-gen-go as dependencies.
In https://github.com/canonical/desktop-engineering/blob/0466f1f6e2ab7044a030731193af9ecabd2c7281/gh-actions/go/generate/action.yaml#L30-L47 we install all the the dependencies from tools/tools.go.
| sudo apt-get install -y ${{ env.build_dependencies }} | ||
|
|
||
| go install honnef.co/go/tools/cmd/staticcheck@latest | ||
| go install google.golang.org/protobuf/cmd/protoc-gen-go@latest |
There was a problem hiding this comment.
In https://github.com/canonical/desktop-engineering/blob/0466f1f6e2ab7044a030731193af9ecabd2c7281/gh-actions/go/generate/action.yaml#L54-L95 we also install protoc. Do we also need that here?
We need to compile the proto files, which means we need the tools used for the compilation.