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

BUD-3634 add input to copy paths to env branch #1

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

alexkhromychenko
Copy link
Collaborator

No description provided.

description: |
Copy specified paths to branch.
Input format: JSON list of objects string with fields: if, source and target.
Example: [{"if": true, "source": "path/to/source1", "target": "path/to/target1"}]

Choose a reason for hiding this comment

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

  • is this is a folder or a file?
  • what is the if for?
  • may it happen that source and target path are different?
  • is this a relative path? relative to what?

can we do something like

 include-paths:
  - source: local/path
    target: target/folder1
  - source: local/path2
    target: target/folder2

can we even copy files to the root target folder?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

is this is a folder or a file?
can be folder or a file

what is the if for?
if is used to add some condition if needed, for example if we want to copy files for wf-budget service but do not want for others

may it happen that source and target path are different?
yes, it's going to be different

is this a relative path? relative to what?
path should be relative to repository root

can we do something like
no, github action input do not support objects, we need to use JSON

can we even copy files to the root target folder?
it should be root of the repo, so should not be any problem

Choose a reason for hiding this comment

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

no, github action input do not support objects, we need to use JSON
you mean Github Actions is yaml based and does not understand yaml lists as parameters?
what about https://stackoverflow.com/questions/65345432/github-actions-pass-list-of-variables-to-shell-script ?

if is used to add some condition if needed, for example if we want to copy files for wf-budget service but do not want for others
oh, right : we would need to add it here https://github.com/OpenGov/k8s-deployment-bnp/blob/main/.github/workflows/render-manifests.yaml#L83 and it would need a per project config so you could do something like

include-paths: [{"if": matrix.env == "wf-budget", "source": "path/to/source1", "target": "path/to/target1"}]

like that?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

you mean Github Actions is yaml based and does not understand yaml lists as parameters?
yaml in general supports lists or object but GH action inputs do not. https://stackoverflow.com/questions/75420197/how-to-use-array-input-for-a-custom-github-actions
I've tried to search in stackoverflow and ask ChatGPT but it only proposes to use JSON

like that?
yes, that's correct

@@ -38,7 +38,7 @@ pushd "${RENDER_DIR}" || exit 1
if [[ -s "${RENDER_FILE}" ]]; then
# Split the rendered file into individual files for each resource
# Invalid GitHub artifact path name characters: Double quote ", Colon :, Less than <, Greater than >, Vertical bar |, Asterisk *, Question mark ?
yq -s '.kind + "-" + (.apiVersion | sub("/", "_")) + "-" + (.metadata.name | sub("[:<>|*?/\\]", "_")) + ".yaml"' < "${RENDER_FILE}"
yq -s '.kind + "-" + (.apiVersion | sub("/", "_")) + "-" + (.metadata.name | sub("[:<>|*?/]", "_")) + ".yaml"' < "${RENDER_FILE}"

Choose a reason for hiding this comment

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

can you elaborate what the change implies and why it was necessary?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Those are changes copied from original repo that we forked from that were added recently

Choose a reason for hiding this comment

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

oh, ok. can you have that separated so we do not mix it up. That way it would also be easy to create PR for the original repo

@alexkhromychenko alexkhromychenko force-pushed the alexkhromychenko/BUD-3634/20240524/trigger_gs_on_commit_status_update branch from 941d10e to 4a52547 Compare May 28, 2024 19:34
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.

2 participants