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

Set the write bit for files written during template initialization #2068

Merged
merged 2 commits into from
Jan 8, 2025

Conversation

pietern
Copy link
Contributor

@pietern pietern commented Jan 2, 2025

Changes

This used to work because the permission bits for built-in templates were hardcoded to 0644 for files and 0755 for directories.

As of #1912 (and the PRs it depends on), built-in templates are no longer pre-materialized to a temporary directory and read directly from the embedded filesystem. This built-in filesystem returns 0444 as the permission bits for the files it contains. These bits are carried over to the destination filesystem.

This change updates template materialization to always set the owner's write bit. It doesn't really make sense to write read-only files and expect users to work with these files in a VCS (note: Git only stores the executable bit).

The regression shipped as part of v0.235.0 and will be fixed as of v0.238.0.

Tests

Unit tests.

This used to work because the permission bits for builtin templates were
hardcoded to 0644 for files and 0755 for directories.

As of #1912 (and the PRs it depends on), builtin templates are no longer
pre-materialized to a temporary directory and read directly from the embedded
filesystem. It turns out that this builtin filesystem returns 0444 as the
permission bits for the files it contains. These bits are carried over to the
destination filesystem.

This change updates template materialization to always set the owner's write
bit. It doesn't really make sense to write read-only files and expect users to
work with these files in a VCS (note: Git only stores the executable bit).

The regression shipped as part of v0.235.0 and will be fixed as of v0.238.0.
Copy link

github-actions bot commented Jan 2, 2025

If integration tests don't run automatically, an authorized user can run them manually by following the instructions below:

Trigger:
go/deco-tests-run/cli

Inputs:

  • PR number: 2068
  • Commit SHA: 6d981c25a472b371c407dc6b3dd46ad3df4ea8f4

Checks will be approved automatically on success.

@pietern pietern temporarily deployed to test-trigger-is January 2, 2025 17:25 — with GitHub Actions Inactive
@pietern pietern enabled auto-merge January 2, 2025 17:28
Copy link
Contributor

@lennartkats-db lennartkats-db left a comment

Choose a reason for hiding this comment

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

👍

@pietern pietern temporarily deployed to test-trigger-is January 8, 2025 12:58 — with GitHub Actions Inactive
@pietern pietern added this pull request to the merge queue Jan 8, 2025
Merged via the queue into main with commit 23f05f5 Jan 8, 2025
9 checks passed
@pietern pietern deleted the fix-write-bit-on-init branch January 8, 2025 13:22
pietern added a commit that referenced this pull request Jan 8, 2025
Bundles:
 * Fix finding Python within virtualenv on Windows ([#2034](#2034)).
 * Include missing field descriptions in JSON schema ([#2045](#2045)).
 * Add validation for volume referenced from `artifact_path` ([#2050](#2050)).
 * Handle `${workspace.file_path}` references in source-linked deployments ([#2046](#2046)).
 * Set the write bit for files written during template initialization ([#2068](#2068)).
github-merge-queue bot pushed a commit that referenced this pull request Jan 8, 2025
Bundles:
* Fix finding Python within virtualenv on Windows
([#2034](#2034)).
* Include missing field descriptions in JSON schema
([#2045](#2045)).
* Add validation for volume referenced from `artifact_path`
([#2050](#2050)).
* Handle `${workspace.file_path}` references in source-linked
deployments ([#2046](#2046)).
* Set the write bit for files written during template initialization
([#2068](#2068)).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants