Failures to run buf lint due to non-lint errors don't fail the action

[carols10cents]

I have a reproducing repo at https://github.com/carols10cents/buf-repro, specifically in carols10cents/buf-repro#3.

This applies to the buf-breaking-action too, but I arbitrarily picked this repo to file the issue in. Let me know if you'd like one over there too.

The .github/workflows/buf-lint.yml contains this (which also runs buf breaking):

name: "Protobuf Lint"
on:
   pull_request:
     types:
       - labeled
       - opened
       - reopened
       - synchronize
jobs:
  lint-protos:
    runs-on: ubuntu-latest
    name: Buf lint
    steps:
      # Run `git checkout`
      - uses: actions/checkout@v4
        if: ${{ !contains(github.event.pull_request.labels.*.name, 'incompatible protobuf') }}
        with:
          fetch-depth: '100'
          submodules: true
      # Install the `buf` CLI
      - uses: bufbuild/buf-setup-action@v1
        if: ${{ !contains(github.event.pull_request.labels.*.name, 'incompatible protobuf') }}
      # Run breaking change detection against the `main` branch
      - uses: bufbuild/buf-breaking-action@v1
        if: ${{ !contains(github.event.pull_request.labels.*.name, 'incompatible protobuf') }}
        with:
          against: 'https://github.com/influxdata/influxdb_iox.git#branch=main'
      # Lint your Protobuf sources
      - uses: bufbuild/buf-lint-action@v1
        if: ${{ !contains(github.event.pull_request.labels.*.name, 'incompatible protobuf') }}

      # Be helpful if the lint fails
      - name: Lint failure! Read this :)
        if: ${{ failure() }}
        run: |
          echo "If you want to make changes forbidden by this lint, please"
          echo "coordinate with the IOx team, add the 'incompatible protobuf' label"
          echo "to the PR, and rerun this test"

The PR reverts a commit that fixed a previous issue with file imports; that is, it breaks again and I expected the action run to fail-- but it passed.

However, toggling open the logs for buf-breaking-action shows:

Run bufbuild/buf-breaking-action@v1
Failure: bulk_ingester/protos/influxdata/iox/bulk_ingester/v1/progress.proto: import "generated_types/protos/influxdata/iox/column_type/v1/type.proto": file does not exist
Failure: bulk_ingester/protos/influxdata/iox/bulk_ingester/v1/progress.proto: import "generated_types/protos/influxdata/iox/column_type/v1/type.proto": file does not exist
No breaking errors were found.

and toggling open the logs for buf-lint-action shows:

Run bufbuild/buf-lint-action@v1
Failure: bulk_ingester/protos/influxdata/iox/bulk_ingester/v1/progress.proto: import "generated_types/protos/influxdata/iox/column_type/v1/type.proto": file does not exist
Failure: bulk_ingester/protos/influxdata/iox/bulk_ingester/v1/progress.proto: import "generated_types/protos/influxdata/iox/column_type/v1/type.proto": file does not exist
No lint errors were found.

I expected these Failure messages to cause the job to fail, because the actions aren't able to actually check these files.

[nicksnyder]

Thank you for the clear reproducible example!

What is happening here is that the incorrect import file is causing the module to not be buildable at all. As a result, the breaking and lint steps report the build errors and fail with a non-zero exit code. For better and for worse, the buf-lint-action and buf-breaking action ignore the build errors and only fail if there are actual breaking or lint errors (they don't count build errors as failing breaking or lint). The build error prevents breaking or lint checks from meaningfully running, so if there is a build error, the lint and breaking checks will always pass.

The quickest solution to your problem is to add an explicit step to your workflow to run buf build to catch build errors. Here is an example.

I also agree/concede that the current behavior of the GitHub actions here is not ideal. We have been working on a revamped unified GitHub action that we think will provide a better end to end experience and holistically address some of the feedback (like this issue) that we have received on our current GitHub actions. If you would be interested to give our new action a spin, we would love feedback: https://github.com/bufbuild/buf-action. Fair warning though, the new action is very new, like we open sourced it last week and we have not yet adopted it across all our own internal repos yet. Having said that, we have also already received positive internal feedback on the new action. It is also where we are investing our time (we don't really plan on making any more changes to the other separate GitHub actions like this one), so if you want to skate to where the puck is heading, and help us make the new action awesome, you are welcome to try it out and let us know what you think!

[nicksnyder]

(The new action correctly addresses this issue both by running the build command by default, and also by ensuring the separate lint/breaking steps fail if the module is not buildable)

[carols10cents]

Awesome, thank you for the PR to add buf build! It exposed yet another mistake I made when creating the reproduction 😬 but I fixed that, merged in the change, and rebased this PR which now fails as I expected! 🎉

The new buf-action looks awesome-- and it seems to be working as I'd expect!! Having everything in one action is awesome -- when we switched from a hand-rolled script to using a GH action for buf checks, we actually had a misconception that the buf-lint-action was also running buf breaking, and it took us like 6 months to notice it wasn't, haha 🤦🏻‍♀️

I'm going to close this issue as either running buf build or using the new buf-action solves the problem. Thank you!!! ❤️