Skip to content

Conversation

ErikJiang
Copy link
Member

@ErikJiang ErikJiang commented Jul 16, 2025

What type of PR is this?

/kind bug

What this PR does / why we need it:

This PR fixes incorrect 0755 (executable) permissions on downloaded archives.

  • A new download_defaults dictionary now sets a safe 0644 mode for archives. Hardcoded permissions were removed from individual items to allow proper inheritance.
  • The owner and mode parameters were removed from the directory creation task. This prevents it from recursively applying incorrect permissions to files already in the destination directory during subsequent loop iterations.

These changes ensure archives receive the correct 0644 permissions, while extracted binaries are correctly set to 0755.

Which issue(s) this PR fixes:

Fixes #12403

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/bug Categorizes issue or PR as related to a bug. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jul 16, 2025
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ErikJiang
Once this PR has been reviewed and has the lgtm label, please assign tico88612 for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jul 16, 2025
@ErikJiang
Copy link
Member Author

/ok-to-test

@k8s-ci-robot k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Jul 21, 2025
@ErikJiang ErikJiang force-pushed the skip_set_mode_owner branch 2 times, most recently from 6e99a30 to 103b889 Compare July 24, 2025 11:21
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 24, 2025
@ErikJiang ErikJiang changed the title Fix(download): Skip setting mode and owner for archives Fix: Correct permissions for downloaded artifacts Jul 24, 2025
@ErikJiang ErikJiang force-pushed the skip_set_mode_owner branch from 103b889 to c23671b Compare July 25, 2025 06:26
dest: "{{ local_release_dir }}/"
remote_src: true
when: container_manager in ['crio', 'containerd']

Copy link
Member

Choose a reason for hiding this comment

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

HI @ErikJiang

Why delete this part ?

Copy link
Member Author

Choose a reason for hiding this comment

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

The unarchive operation is already configured via downloads.etcd.unarchive: true, so the extraction in extract_file.yml will handle it automatically.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. release-note-none Denotes a PR that doesn't merit a release note. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Downloaded tarballs don't need root ownership and execute bit
3 participants