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.

Sign up for GitHub

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

Jump to bottom

ci: fix release step by separating release to AWS bucket. #1834

Merged
merged 1 commit into from
Jun 19, 2024
Merged

ci: fix release step by separating release to AWS bucket. #1834

merged 1 commit into from
Jun 19, 2024

Conversation

Matovidlo
Copy link
Contributor

@Matovidlo Matovidlo commented Jun 12, 2024 •
edited by jira bot
Loading

Jira: PSGO-668

Changes:

  • The new cache located in AWS has changed the credentials of workflow and is unable to run post save workflow.

@Matovidlo Matovidlo force-pushed the ci-release-fix branch 8 times, most recently from 8306b25 to 972232a Compare June 12, 2024 09:56
@Matovidlo
Copy link
Contributor Author

@michaljurecko @hosekpeter I could not reach the success of release step in using make release. However the AWS credentials seems to be located elsewhere which should not cause trouble in the workflow anymore.

@Matovidlo Matovidlo marked this pull request as ready for review June 12, 2024 09:59
@michaljurecko michaljurecko removed their request for review June 12, 2024 10:28
@michaljurecko
Copy link
Contributor

(I think you will manage without me, if you have a specific question, I will answer)

jachym-tousek-keboola
jachym-tousek-keboola requested changes Jun 14, 2024
View reviewed changes
Copy link
Contributor

@jachym-tousek-keboola jachym-tousek-keboola left a comment

Choose a reason for hiding this comment

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

I'm confused. Separating the release to separate file should not have an effect on its own, right? What did you change in the configuration that actually fixed it?

.github/workflows/release-cli.yml Outdated
Comment on lines 447 to 463

Copy link
Contributor

Choose a reason for hiding this comment

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

?

Copy link
Contributor Author

@Matovidlo Matovidlo Jun 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

trailing spaces. I think we have very bad convention on how to write CI pipeline, each file looks different. I will though about some yaml linter that enforces us to write it correctly, otherwise each PR will look like this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#1853 Added draft PR which sets the yaml linter. I would resolve this for now and adjust all yaml code so it is correctly written. (Maintanance task)

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't particularly feel like we need a linter for that. Just set up your ide to respect .editorconfig.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you remove the trailing whitepaces please? That's a problem of your editor mainly.

Copy link
Contributor Author

@Matovidlo Matovidlo Jun 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

You mean add trailing whitespaces? Are you sure? 😅 When I have opened editor and being on that line no space is there. I think the issue lays in git diff. It shows wrong

Copy link
Contributor

Choose a reason for hiding this comment

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

I checked locally. The diff is correct, you did add trailing whitespaces there. Force-pushing a fix.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I pulled it and see no changes. Weird

@Matovidlo
Copy link
Contributor Author

The step of aws-actions/configure-aws-credentials@v4 has side effects that affects other workflows. As it is separate it uses sort of local environment in which the configuration of various credentails do not affect others. It should solve this issue https://github.com/keboola/keboola-as-code/actions/runs/9478829184/job/26117435752

@hosekpeter
Copy link
Contributor

It's interesting that if it's separated it solves the problem. we have some slack 🧵 where i can learn more about it. @Matovidlo

@Matovidlo Matovidlo force-pushed the ci-release-fix branch 9 times, most recently from 6dfefff to 5017389 Compare June 18, 2024 09:52
jachym-tousek-keboola
jachym-tousek-keboola requested changes Jun 18, 2024
View reviewed changes
Copy link
Contributor

@jachym-tousek-keboola jachym-tousek-keboola left a comment

Choose a reason for hiding this comment

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

Considering one of the commits is names "WIP: test CI" I'm guessing it's not complete yet? Or maybe you just need to fix the commit message?

@Matovidlo Matovidlo force-pushed the ci-release-fix branch 19 times, most recently from a703349 to e87a07f Compare June 19, 2024 09:12
jachym-tousek-keboola
jachym-tousek-keboola requested changes Jun 19, 2024
View reviewed changes
.github/workflows/release-cli.yml Outdated
Comment on lines 447 to 463

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you remove the trailing whitepaces please? That's a problem of your editor mainly.

@Matovidlo @jachym-tousek-keboola
ci: separate release step and create step to push artefacts into AWS …
c77d2b2 
…bucket.

1. Calling `aws-actions/configure-aws-credentials` with ` in combination with `./.github/actions/go-setup` does not work. Post workflow is corrupted due to `missing AWS region`
2. Release step now does not have `publisher` in goreleaser. The release
   script is called in separate CI step.
3. Release step creates new artefacts that will be published.
3. This separate CI step has `aws-actions.configure-aws-credentials` and
   cause no problems. This step dowloads published release artefacts and
publish it to both s3 and github release.
@Matovidlo Matovidlo merged commit 13a2f35 into main Jun 19, 2024
13 checks passed
@Matovidlo Matovidlo deleted the ci-release-fix branch June 19, 2024 11:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jachym-tousek-keboola jachym-tousek-keboola jachym-tousek-keboola approved these changes

@hosekpeter hosekpeter Awaiting requested review from hosekpeter hosekpeter is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Matovidlo @michaljurecko @hosekpeter @jachym-tousek-keboola