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

Fix pr issues #2968

Merged
merged 7 commits into from
Jun 20, 2024
Merged

Fix pr issues #2968

merged 7 commits into from
Jun 20, 2024

Conversation

joaopapereira
Copy link
Contributor

Description of the Change

Fix github action failing while testing PRs. We found out that when we were using workflow_run, we were always retrieving the workflow files from the main branch. The main change that is made here is to stop relying on workflow_run and to call the integration tests after the unit tests are successful.

Copy link
Member

@moleske moleske left a comment

Choose a reason for hiding this comment

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

I like this flow better cause then the branches won't be dependent on main. That has been tricking people for awhile.

I'm approving cause this flow seems reasonable. I'm assuming the errors about Incorrect number of arguments provided will be resolved before merging. Also not sure if this is one of the cases where stuff needs to be merged first before seeing it work (that was often the case with the previous method of getting the workflow from the main branch)

@joaopapereira joaopapereira force-pushed the fix-pr-issues branch 2 times, most recently from d3be858 to d89ff08 Compare June 12, 2024 14:56
We were having issues when calling a workflow from a workflow, the
secrest did not look like they were being passed along. This change
explicitly sets the secrets that are being passed

Signed-off-by: João Pereira <[email protected]>
@a-b
Copy link
Member

a-b commented Jun 12, 2024

The integration tests workflow was designed in that specific way to enable PRs from forks to run integration tests using repository secrets containing credentials for the Shepherd. It also requires approval to run tests for new contributors, which helps maintain security for sensitive information.

Do we know if the proposed change operates in the equivalent way?

@joaopapereira
Copy link
Contributor Author

The only change this PR introduces is how the integration tests are triggered. Instead of using a "global" trigger, each workflow explicitly runs the integration tests, which is the only change.

Starting on version 1.181.0, capi will no longer report the version of
the nginx server to ensure that no information is leaked.
For more information check cloudfoundry/capi-release#406

Signed-off-by: João Pereira <[email protected]>
Starting on version 76.26.0 of UAA a change was made that changes the
behavior more context in cloudfoundry/uaa#2545

Signed-off-by: João Pereira <[email protected]>
Copy link
Member

@gururajsh gururajsh left a comment

Choose a reason for hiding this comment

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

LGTM

@gururajsh gururajsh merged commit 36abb81 into cloudfoundry:main Jun 20, 2024
11 of 14 checks passed
Copy link
Member

@a-b a-b left a comment

Choose a reason for hiding this comment

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

LGTM

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