Skip to content

fix: Limit TOML file size#3432

Merged
sfc-gh-jmichalak merged 6 commits into
devfrom
file-size
Mar 13, 2025
Merged

fix: Limit TOML file size#3432
sfc-gh-jmichalak merged 6 commits into
devfrom
file-size

Conversation

@sfc-gh-jmichalak

@sfc-gh-jmichalak sfc-gh-jmichalak commented Mar 10, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor
  • Limit TOML file size to 10MB.
  • Move common OS-related functions to a wrapper package.
  • Log interactions with OS.

Test Plan

  • acceptance tests
  • …

TODO

Check the behavior on Windows and adjust the tests.

@github-actions

github-actions Bot commented Mar 10, 2025

Copy link
Copy Markdown

Integration tests cancelled for c4d76cb62803de5b0d743fdb1219001be1769d58

@github-actions

github-actions Bot commented Mar 10, 2025

Copy link
Copy Markdown

Integration tests failure for fa5a7e0a5c8f7fc5e68e2c766e95960eb9cbcd29

@github-actions

github-actions Bot commented Mar 11, 2025

Copy link
Copy Markdown

Integration tests cancelled for 2fe017070d18c0ded84fbb424eca9da29a2f938a

@github-actions

github-actions Bot commented Mar 11, 2025

Copy link
Copy Markdown

Integration tests failure for 3bb0f461a05958de88bf720cc097cf29fbb39121

sfc-gh-asawicki
sfc-gh-asawicki requested changes Mar 11, 2025
View reviewed changes
Comment thread MIGRATION_GUIDE.md
Comment thread pkg/internal/os/os.go
Comment thread pkg/internal/os/os_test.go Outdated
)

func TestLoadConfigFileThatIsTooBig(t *testing.T) {
if os.IsRunningOnWindows {

@sfc-gh-asawicki sfc-gh-asawicki Mar 11, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this if seems to be not needed (if it works on windows - so let's check the manual tests)

@sfc-gh-jmichalak sfc-gh-jmichalak Mar 12, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Probably, my plan is to merge this and permissions and check all at once on a Windows machine. If it's possible, I'll remove that.

@sfc-gh-asawicki sfc-gh-asawicki Mar 12, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ok, keeping it open then.

Comment thread pkg/internal/os/os_test.go
@@ -0,0 +1,27 @@
package os_test

import (

@sfc-gh-asawicki sfc-gh-asawicki Mar 11, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: all methods should have tests (at least smoke ones to check if the output in simple case is the same). Can be done later.

@sfc-gh-jmichalak sfc-gh-jmichalak Mar 12, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'll do this later with permissions.

@sfc-gh-asawicki sfc-gh-asawicki Mar 12, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ok, keeping it open then.

Comment thread pkg/internal/os/os.go
Comment thread pkg/provider/provider_acceptance_test.go Outdated
Comment thread pkg/sdk/config.go Outdated
@sfc-gh-asawicki sfc-gh-asawicki self-requested a review March 12, 2025 13:49
@github-actions

github-actions Bot commented Mar 12, 2025

Copy link
Copy Markdown

Integration tests failure for 3fd2d3ae861504138c61827de1134b4eb21ade72

@sfc-gh-jmichalak sfc-gh-jmichalak merged commit 2e12981 into dev Mar 13, 2025
@sfc-gh-jmichalak sfc-gh-jmichalak deleted the file-size branch March 13, 2025 09:21
sfc-gh-jmichalak added a commit that referenced this pull request Mar 24, 2025
@sfc-gh-jmichalak
fix: Limit TOML file size (#3432)
55fcbe3 
<!-- Feel free to delete comments as you fill this in -->
- Limit TOML file size to 10MB.
- Move common OS-related functions to a wrapper package.
- Log interactions with OS.
<!-- summary of changes -->

## Test Plan
<!-- detail ways in which this PR has been tested or needs to be tested
-->
* [x] acceptance tests
<!-- add more below if you think they are relevant -->
* [ ] …

## TODO
Check the behavior on Windows and adjust the tests.
@snowflake-release-please snowflake-release-please Bot mentioned this pull request Mar 24, 2025
Merged
sfc-gh-jmichalak pushed a commit that referenced this pull request Mar 25, 2025
@snowflake-release-please
chore(main): release 1.0.5 (#3463)
3f8e4a2 
🤖 I have created a release *beep* *boop*
---


##
[1.0.5](v1.0.4...v1.0.5)
(2025-03-25)


### 🔧 **Misc**

* Bump tracking version to v1.0.5
([#3460](#3460))
([2af79e6](2af79e6))
* Bump tracking version to v1.0.5
([#3460](#3460))
([3aaf555](3aaf555))
* Document godebug flag usage
([#3429](#3429))
([5f04947](5f04947))
* Document godebug flag usage
([#3429](#3429))
([2dce172](2dce172))
* fix failing tests
([#3428](#3428))
([884ca54](884ca54))
* fix failing tests
([#3428](#3428))
([245fd86](245fd86))
* Fix migration guide
([#3468](#3468))
([6540841](6540841))
* GitHub actions cleanup
([#3431](#3431))
([1cc6d80](1cc6d80))
* GitHub actions cleanup
([#3431](#3431))
([a6e7429](a6e7429))
* Protect pentesting user and role from sweepers
([#3426](#3426))
([1478c5e](1478c5e))
* Protect pentesting user and role from sweepers
([#3426](#3426))
([6354dba](6354dba))
* Remove driver instrumentation
([#3439](#3439))
([61802a8](61802a8))
* Remove driver instrumentation
([#3439](#3439))
([e9fc6ed](e9fc6ed))
* Remove SF_TF_ADDITIONAL_DEBUG_LOGGING
([#3441](#3441))
([cab63e9](cab63e9))
* Remove SF_TF_ADDITIONAL_DEBUG_LOGGING
([#3441](#3441))
([b57db81](b57db81))
* Update GitHub actions
([#3455](#3455))
([3b027a8](3b027a8))
* Update GitHub actions
([#3455](#3455))
([368f090](368f090))
* Use environmental secrets in GitHub Actions
([#3440](#3440))
([6048977](6048977))
* Use environmental secrets in GitHub Actions
([#3440](#3440))
([8e5a262](8e5a262))


### 🐛 **Bug fixes:**

* Add boolean env validations and unit tests for TOML config validation
([#3453](#3453))
([c113928](c113928))
* Add boolean env validations and unit tests for TOML config validation
([#3453](#3453))
([408db4e](408db4e))
* Adjust docs
([#3451](#3451))
([46d071d](46d071d))
* Adjust docs
([#3451](#3451))
([29b0e4d](29b0e4d))
* Apply new assertions setup
([#3409](#3409))
([e5fdced](e5fdced))
* Apply new assertions setup
([#3409](#3409))
([af83da0](af83da0))
* Correct a typo in a tag resource example
([#3446](#3446))
([a0202d1](a0202d1))
* Correct a typo in a tag resource example
([#3446](#3446))
([afd19a2](afd19a2))
* Fix datasource acceptance tests setup part1
([#3443](#3443))
([58126ea](58126ea))
* Fix datasource acceptance tests setup part1
([#3443](#3443))
([5046654](5046654))
* Fix datasource acceptance tests setup part2
([#3445](#3445))
([6c7ceeb](6c7ceeb))
* Fix datasource acceptance tests setup part2
([#3445](#3445))
([4ba2bb6](4ba2bb6))
* Fix datasource acceptance tests setup part3
([#3448](#3448))
([5475c69](5475c69))
* Fix datasource acceptance tests setup part3
([#3448](#3448))
([f6fdc17](f6fdc17))
* Fix datasource acceptance tests setup part4
([#3449](#3449))
([04e082a](04e082a))
* Fix datasource acceptance tests setup part4
([#3449](#3449))
([935d969](935d969))
* Fix tasks tests
([#3434](#3434))
([33ab740](33ab740))
* Fix tasks tests
([#3434](#3434))
([b5457f6](b5457f6))
* Handle TOML file permissions
([#3444](#3444))
([6c746b4](6c746b4))
* Handle TOML file permissions
([#3444](#3444))
([e4ed171](e4ed171))
* Limit TOML file size
([#3432](#3432))
([55fcbe3](55fcbe3))
* Limit TOML file size
([#3432](#3432))
([2e12981](2e12981))
* Quick fix assertions
([#3438](#3438))
([0e05c6f](0e05c6f))
* Quick fix assertions
([#3438](#3438))
([a430ba7](a430ba7))
* Skip file permission verification by default
([#3476](#3476))
([562173d](562173d))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: snowflake-release-please[bot] <105954990+snowflake-release-please[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sfc-gh-jcieslak sfc-gh-jcieslak sfc-gh-jcieslak approved these changes

@sfc-gh-asawicki sfc-gh-asawicki sfc-gh-asawicki approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@sfc-gh-jmichalak @sfc-gh-jcieslak @sfc-gh-asawicki