Skip to content

Using new version of setup-kind using new add-path mechanism#11420

Closed
potiuk wants to merge 1 commit intoapache:masterfrom
PolideaInternal:fix-deprecation-warning-kind
Closed

Using new version of setup-kind using new add-path mechanism#11420
potiuk wants to merge 1 commit intoapache:masterfrom
PolideaInternal:fix-deprecation-warning-kind

Conversation

@potiuk
Copy link
Member

@potiuk potiuk commented Oct 11, 2020

Add Path was deprecated in
https://github.blog/changelog/2020-10-01-github-actions-deprecating-set-env-and-add-path-commands/
and will be removed soon, so this PR changes the mechanism to
the new one.

The path is added to the ${GITHUB_PATH} file instead as described in
https://docs.github.com/en/free-pro-team@latest/actions/reference/workflow-commands-for-github-actions#adding-a-system-path

For now this was (hopefully) fixed in potiuk's fork but the
plan is to make PR to the original action and use it from
there once it is merged.


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.

Add Path was deprecated in
https://github.blog/changelog/2020-10-01-github-actions-deprecating-set-env-and-add-path-commands/
and will be removed soon, so this PR changes the mechanism to
the new one.

The path is added to the ${GITHUB_PATH} file instead as described in
https://docs.github.com/en/free-pro-team@latest/actions/reference/workflow-commands-for-github-actions#adding-a-system-path

For now this was (hopefully) fixed in potiuk's fork but the
plan is to make PR to the original action and use it from
there once it is merged.

Also in the original action the lib/*.js files were ignored and
not present in the repository, which made it impossible to refer
to the action via commit hash as recommended in

https://docs.github.com/en/free-pro-team@latest/actions/learn-github-actions/security-hardening-for-github-actions#using-third-party-actions

The change in potiuk's repository adds the files to lib and
makes it possible to refer to the action via commit hash.
@potiuk potiuk force-pushed the fix-deprecation-warning-kind branch from f9e2d51 to a4442bb Compare October 11, 2020 17:42
@potiuk
Copy link
Member Author

potiuk commented Oct 11, 2020

Seems it needs more work :(. The setup-kind action is not well prepared for serving directly from the repo.

@potiuk
Copy link
Member Author

potiuk commented Oct 11, 2020

Possibly we should even change the action.

run: ./scripts/ci/tools/ci_free_space_on_ci.sh
- name: "Setup Kind Cluster ${{ env.KIND_VERSION }}"
uses: engineerd/setup-kind@v0.4.0
uses: potiuk/setup-kind@7345a7e015d9844bab5f4a8a8cfb4eb3cf7f91ef
Copy link
Member

Choose a reason for hiding this comment

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

Did you pr this upstream? If so could you add a comment to the yaml saying "change back when X is merged"

Copy link
Member Author

Choose a reason for hiding this comment

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

Not yet. But I plan to do it when I get it working (for now I opened the issue engineerd/setup-kind#28). For now, it seems that the way they implemented it, we cannot even link to the commit hash for security (seems that you can only link to packaged .zip uploaded as a package which is not really what you want).
I added generated .js code back to the repo (it was .gitignored) but it's not enough - their npm run pack does not create a fully working and minified .js. I had no time to investigate but either I will make it "properly" and PR back (and they hopefully approve) or we find another action for it..

Copy link
Member

Choose a reason for hiding this comment

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

Oh ugh :(

@potiuk potiuk marked this pull request as draft October 11, 2020 19:16
@potiuk
Copy link
Member Author

potiuk commented Oct 11, 2020

Converted it to Draft for now.

@potiuk potiuk closed this Oct 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants