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

Upcoming breaking change: Hidden Files will be excluded by default #602

Closed
joshmgross opened this issue Aug 19, 2024 · 60 comments
Closed

Upcoming breaking change: Hidden Files will be excluded by default #602

joshmgross opened this issue Aug 19, 2024 · 60 comments
Assignees

Comments

@joshmgross
Copy link
Member

joshmgross commented Aug 19, 2024

From September 2nd, 2024, we will no longer include hidden files and folders as part of the default upload of the v3 and v4 upload-artifact actions. This reduces the risk that credentials are accidentally uploaded into artifacts. Customers who need to continue to upload these files can use a new option, ‘include-hidden-files’, to continue to do so.

https://github.blog/changelog/2024-08-19-notice-of-upcoming-deprecations-and-breaking-changes-in-github-actions-runners/

@dolfinus
Copy link

dolfinus commented Sep 2, 2024

Why introducing breaking changes to v3 and v4 instead of making a new v5 version?

@uerkut
Copy link

uerkut commented Sep 2, 2024

      - name: Upload Docker Compose Files Artifact
        id: upload_docker_compose_files
        uses: actions/[email protected]
        include-hidden-files: true
        with:
          name: docker-compose-files
          path: |
            .env
            docker-compose.yaml

I've edited my workflow like above and getting the error:

The workflow is not valid. In .github/workflows/kondukto-package-build-manual.yml (Line: 318, Col: 11): Error from called workflow kondukto-io/kondukto-build/.github/workflows/docker_compose_package.yml@c343cea5db32d5d329cfe25ea5f7f84a88ecb64a (Line: 38, Col: 9): Unexpected value 'include-hidden-files'

I tried both versions v4 and v4.4.0

@dolfinus
Copy link

dolfinus commented Sep 2, 2024

I've edited my workflow like above and getting the error:

You should place include-hidden-files: true under with: block

@uerkut
Copy link

uerkut commented Sep 2, 2024

Thanks a lot. My bad.

robertknight added a commit to hypothesis/h that referenced this issue Sep 2, 2024
The `upload-artifact` action recently introduced a change where by default it no
longer uploads hidden files. This affected the `.coverage.*` files produced by
Python tests. There is an `include-hidden-files: true` option to change this,
but so far in our testing it did not work. Fix the issue by changing the file
name to be non-hidden.

See actions/upload-artifact#602 and
https://hypothes-is.slack.com/archives/C4K6M7P5E/p1725285976308269.
@catYalere
Copy link

image
image
Funny thing [in](include-hidden-files: true) doesn't work on v3, in one screenshoot I point directly to file and also doing a cat to see if it exists and exists

@buckett
Copy link

buckett commented Sep 2, 2024

This change resulted in deployment failures for us because we no longer included the full set of files in our deployment artifact.

@Drowze
Copy link

Drowze commented Sep 2, 2024

This also resulted in a lot of CI failures for us... Would be nice if there was at least a warning before rolling breaking changes 🤷‍♂️

Here's how we were calling this action:

    - uses: actions/upload-artifact@v4
      with:
        name: artifacts-${{ strategy.job-index }}
        path: |-
          coverage/.resultset.json
          tests/junit.xml

@joshmgross
Copy link
Member Author

Funny thing [in](include-hidden-files: true) doesn't work on v3, in one screenshoot I point directly to file and also doing a cat to see if it exists and exists

👋 Thanks for the report, we're rolling back the v3 and v3-node20 updates while we work on fixing that issue.

robertknight added a commit to hypothesis/h that referenced this issue Sep 2, 2024
The `upload-artifact` action recently introduced a change where by default it no
longer uploads hidden files. This affected the `.coverage.*` files produced by
Python tests. There is an `include-hidden-files: true` option to change this,
but so far in our testing it did not work. Fix the issue by changing the file
name to be non-hidden.

See actions/upload-artifact#602 and
https://hypothes-is.slack.com/archives/C4K6M7P5E/p1725285976308269.
@gurvancampion
Copy link

Same here, I suddenly got an issue in my CI while deploying my app that never occurred before:
CleanShot 2024-09-02 at 18 30 54@2x

@tpope
Copy link

tpope commented Sep 2, 2024

We also got burned by this. I see the logic in excluding hidden files recursively nested inside directories. But I struggle to see the value in ignoring an explicit path: .dotfile parameter.

@joshmgross
Copy link
Member Author

Funny thing [in](include-hidden-files: true) doesn't work on v3, in one screenshoot I point directly to file and also doing a cat to see if it exists and exists

👋 Thanks for the report, we're rolling back the v3 and v3-node20 updates while we work on fixing that issue.

This is fixed in #608 & #609

https://github.com/actions/upload-artifact/releases/tag/v3.2.1
https://github.com/actions/upload-artifact/releases/tag/v3.2.1-node20

We're updating the major version tags v3 & v3-node20 to include those fixes.

@catYalere
Copy link

TY v3 with flag working
image

@antoniocoratelli
Copy link

Breaking changes should bump major version! What kind of semantic versioning is this?

And if old behavior poses security problems, issue a deprecation warning via EMAIL to organization owners that have repos using this very important action.

Breaking developers workflows on such a short notice (if writing a blog post can be considered an actual "notice") is NOT OK

@supertr0n
Copy link

The combined wasted person-hours and other indirect costs caused by this very unexpected breaking change must be quite large.

It's worth considering how many of GitHub's customers' own urgent security patches/updates for their own software have been held up from being deployed because of this random breakage of their CI pipeline.

@emmahsax
Copy link

emmahsax commented Sep 3, 2024

Why introducing breaking changes to v3 and v4 instead of making a new v5 version?

To echo this sentiment, there should not have been a breaking change without a new major version. This change literally caused downtime on our production system and took us hours to debug (we had rolled back production by then of course).

Even if there were security concerns, the default should've been true in v3 and v4, false in v5, and then new v3 and v4 minor or patch updates which could write an annotation to GitHub to say that due to security concerns, the team recommends users add include-hidden-files: false in their workflows.

This solution wouldn't have broken anything for anybody, while also notifying them more to the fact that there's recommendations.

Also, totally icky and rude to be releasing this on a day when many people are not working due to a national holiday. It's almost like they wanted to just slide it in when people may not be paying attention..... 🙄

jni pushed a commit to napari/napari that referenced this issue Sep 3, 2024
# References and relevant issues

actions/upload-artifact#602

https://napari.zulipchat.com/#narrow/stream/348229-randoms/topic/actions.2Fdownload-artifact/near/467017518

# Description

Someone forget what semantic versioning means, so we need to try to deal
with it. Hopefully found all places.
@Pixelatex
Copy link

@nebuk89 can we send you the invoice for downtime and time spent because this didn't break the pipeline but instead deployed applications without needed files to run..

Acknowledge your mistake and don't hide behind the shield that you somehow saved the day by shielding us from the big bad evil hidden files upload... Seriously, go touch grass.. this is not some zero-day you're patching, you do not ignore semver and push a breaking change for such a stupid reason.

@smokedlinq
Copy link

Why not publish a v5 version for this? This is breaking a ton of our things that build files with .folders such as Azure Functions.

@josheche
Copy link

josheche commented Sep 3, 2024

We still use v3 of upload-artifacts. My scheduled pipelines broke last night silently because it was looking for our .env files but needed include-hidden-files to be set to true instead of the default false that was supposed to be a breaking change for v4.

@actions actions locked as too heated and limited conversation to collaborators Sep 3, 2024
AndyRae added a commit to Health-Informatics-UoN/carrot-mapper that referenced this issue Sep 5, 2024
twm added a commit to twisted/incremental that referenced this issue Sep 6, 2024
carltongibson added a commit to carltongibson/django-filter that referenced this issue Sep 16, 2024
skshetry added a commit to iterative/dvc-bench that referenced this issue Sep 17, 2024
Joshix-1 added a commit to Joshix-1/typed_stream that referenced this issue Sep 29, 2024
Joshix-1 added a commit to asozialesnetzwerk/an-website that referenced this issue Sep 29, 2024
toddsfish added a commit to toddsfish/cdk-autoscaling-warmpool that referenced this issue Oct 1, 2024
toddsfish added a commit to toddsfish/cdk-autoscaling-warmpool that referenced this issue Oct 1, 2024
ahmedaljawahiry added a commit to ahmedaljawahiry/django-admin-site-search that referenced this issue Nov 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests