From 36abb8198be118296d97ba321e163c17c72846ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Pereira?= Date: Thu, 20 Jun 2024 09:48:55 -0500 Subject: [PATCH] Fix pr issues (#2968) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Ensure correct pool is being used for PRs * Use integration workflow directly from unit tests * Provide secret directly instead of using env variable * Remove check for Server header in curl request tests 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 https://github.com/cloudfoundry/capi-release/pull/406 * Change in response from UAA Starting on version 76.26.0 of UAA a change was made that changes the behavior more context in https://github.com/cloudfoundry/uaa/issues/2545 Signed-off-by: João Pereira --- .../workflows/tests-integration-reusable.yml | 39 +++---------------- .github/workflows/tests-integration.yml | 27 ++++++------- .github/workflows/tests-unit.yml | 10 +++++ api/uaa/error_converter.go | 2 +- api/uaa/error_converter_test.go | 16 ++++++++ integration/v7/isolated/curl_command_test.go | 2 - 6 files changed, 45 insertions(+), 51 deletions(-) diff --git a/.github/workflows/tests-integration-reusable.yml b/.github/workflows/tests-integration-reusable.yml index e8a80a5915..cff651d035 100644 --- a/.github/workflows/tests-integration-reusable.yml +++ b/.github/workflows/tests-integration-reusable.yml @@ -27,11 +27,7 @@ on: default: ${{ vars.SHEPHERD_POOL_NAME }} pool-namespace: type: string - default: 'tas-devex' - is-pr: - type: boolean - default: true - + default: 'official' jobs: run-integration-tests: defaults: @@ -40,17 +36,6 @@ jobs: runs-on: ${{ inputs.os }} container: us-west2-docker.pkg.dev/shepherd-268822/shepherd2/concourse-resource:latest steps: - - uses: LouisBrunner/checks-action@v2.0.0 - if: always() && inputs.is-pr - id: check - with: - token: ${{ secrets.GITHUB_TOKEN }} - name: "${{ inputs.name }}" - status: in_progress - sha: ${{github.event.workflow_run.head_sha}} - output: | - {"title": "${{ inputs.name }}", "summary":"started ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}"} - - name: Checkout cli uses: actions/checkout@v4 with: @@ -100,16 +85,16 @@ jobs: run: | shepherd login service-account ${account_token} - echo "shepherd create lease --duration 8h --pool ${pool_name} --pool-namespace tas-devex --namespace ${pool_namespace}" - lease_id=$(shepherd create lease --duration 8h --pool ${pool_name} --pool-namespace tas-devex --namespace ${pool_namespace} --json | jq -r .id) + echo "shepherd create lease --duration 8h --pool ${pool_name} --pool-namespace ${pool_namespace} --namespace tas-devex" + lease_id=$(shepherd create lease --duration 8h --pool ${pool_name} --pool-namespace ${pool_namespace} --namespace tas-devex --json | jq -r .id) # Give sometime for the lease to complete. Shepherd may take upto an 3 hours to create an env # if the pool is empty. count=0 while [ $count -lt 360 ] ; do sleep 30 - status=$(shepherd get lease ${lease_id} --namespace ${pool_namespace} --json | jq -r .status) + status=$(shepherd get lease ${lease_id} --namespace tas-devex --json | jq -r .status) if [ $status == "LEASED" ] ; then - shepherd get lease ${lease_id} --namespace ${pool_namespace} --json | jq .output > metadata.json + shepherd get lease ${lease_id} --namespace tas-devex --json | jq .output > metadata.json break elif [ $status == "FAILED" -o $status == "EXPIRED" ] ; then echo "There was an error obtaining the lease. Lease status is ${status}." @@ -157,8 +142,6 @@ jobs: - name: Deploy Isolation Segment and OIDC Provider if: ${{ inputs.capi-version == 'edge' }} - env: - CF_INT_CLIENT_SECRET: ${{ secrets.CLIENT_SECRET }} run: | env_name=$(jq -r .name metadata.json) jq -r .bosh.jumpbox_private_key metadata.json > /tmp/${env_name}.priv @@ -172,7 +155,7 @@ jobs: -o cli-ci/ci/infrastructure/operations/add-oidc-provider.yml \ -o cli-ci/ci/infrastructure/operations/add-uaa-client-credentials.yml \ -o cli-ci/ci/infrastructure/operations/diego-cell-instances.yml \ - -v client-secret="${CF_INT_CLIENT_SECRET}" \ + -v client-secret="${{ secrets.CLIENT_SECRET }}" \ > ./director.yml bosh -d cf deploy director.yml -n @@ -272,13 +255,3 @@ jobs: shepherd login service-account ${account_token} set -x shepherd delete lease ${{ steps.claim-env.outputs.lease-id }} --namespace tas-devex - - - uses: LouisBrunner/checks-action@v2.0.0 - if: always() && inputs.is-pr - with: - token: ${{ secrets.GITHUB_TOKEN }} - check_id: ${{ steps.check.outputs.check_id }} - conclusion: ${{ job.status }} - sha: ${{github.event.workflow_run.head_sha}} - output: | - {"title": "${{ inputs.name }}", "summary":"finished ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}"} diff --git a/.github/workflows/tests-integration.yml b/.github/workflows/tests-integration.yml index baa7a4ad2a..a19db18c1d 100644 --- a/.github/workflows/tests-integration.yml +++ b/.github/workflows/tests-integration.yml @@ -3,6 +3,11 @@ name: "Tests: Integration" run-name: "Integration [${{ github.event.workflow_run.head_branch }}]: ${{ github.event.workflow_run.head_commit.message }}" on: + workflow_call: + inputs: + workflow: + default: all + type: string workflow_dispatch: inputs: workflow: @@ -15,40 +20,32 @@ on: - run-integration-tests-cf-env-with-min-capi # - run-integration-windows # - run-integration-windows-client-credentials - workflow_run: - workflows: - - "Tests" - types: - - completed - -jobs: +jobs: run-integration-tests-cf-env: name: Integration tests - if: ${{ (github.event_name == 'workflow_dispatch' && inputs.workflow == 'run-integration-tests-cf-env') || github.event.workflow_run.conclusion == 'success' }} + if: ${{ inputs.workflow == 'all' || inputs.workflow == 'run-integration-tests-cf-env' }} uses: ./.github/workflows/tests-integration-reusable.yml with: capi-version: edge run-with-client-creds: false os: ubuntu-latest name: Integration - is-pr: ${{ github.event_name != 'workflow_dispatch' }} secrets: inherit run-integration-tests-cf-env-with-client-creds: name: client creds - if: ${{ (github.event_name == 'workflow_dispatch' && inputs.workflow == 'run-integration-tests-cf-env-with-client-creds') || github.event.workflow_run.conclusion == 'success' }} + if: ${{ inputs.workflow == 'all' || inputs.workflow == 'run-integration-tests-cf-env-with-client-creds' }} uses: ./.github/workflows/tests-integration-reusable.yml with: capi-version: edge run-with-client-creds: true os: ubuntu-latest name: Integration client creds - is-pr: ${{ github.event_name != 'workflow_dispatch' }} secrets: inherit run-integration-tests-cf-env-with-min-capi: name: MIN CAPI - if: ${{ (github.event_name == 'workflow_dispatch' && inputs.workflow == 'run-integration-tests-cf-env-with-min-capi') || github.event.workflow_run.conclusion == 'success' }} + if: ${{ inputs.workflow == 'all' || inputs.workflow == 'run-integration-tests-cf-env-with-min-capi' }} uses: ./.github/workflows/tests-integration-reusable.yml with: capi-version: min @@ -56,12 +53,12 @@ jobs: os: ubuntu-latest name: Integration MIN CAPI pool-name: cfd_16_11_0 - is-pr: ${{ github.event_name != 'workflow_dispatch' }} + pool-namespace: tas-devex secrets: inherit #run-integration-windows: # name: Windows - # if: ${{ (github.event_name == 'workflow_dispatch' && inputs.workflow == 'run-integration-windows') || github.event.workflow_run.conclusion == 'success' }} + # if: ${{ inputs.workflow == 'all' || inputs.workflow == 'run-integration-windows' }} # uses: ./.github/workflows/tests-integration-reusable.yml # with: # capi-version: edge @@ -72,7 +69,7 @@ jobs: #run-integration-windows-client-credentials: # name: Windows with client credentials - # if: ${{ (github.event_name == 'workflow_dispatch' && inputs.workflow == 'run-integration-windows-client-credentials') || github.event.workflow_run.conclusion == 'success' }} + # if: ${{ inputs.workflow == 'all' || inputs.workflow == 'run-integration-windows-client-credentials' }} # uses: ./.github/workflows/tests-integration-reusable.yml # with: # capi-version: edge diff --git a/.github/workflows/tests-unit.yml b/.github/workflows/tests-unit.yml index 51b23b8132..0301a89b4e 100644 --- a/.github/workflows/tests-unit.yml +++ b/.github/workflows/tests-unit.yml @@ -84,4 +84,14 @@ jobs: Get-Item Makefile make units + + + integration: + needs: + - units + - units-windows + name: Integration tests + if: ${{ github.event != 'workflow_dispatch' }} + uses: ./.github/workflows/tests-integration.yml + secrets: inherit # vim: set sw=2 ts=2 sts=2 et tw=78 foldlevel=2 fdm=indent nospell: diff --git a/api/uaa/error_converter.go b/api/uaa/error_converter.go index ebbe950cae..c5ecb29921 100644 --- a/api/uaa/error_converter.go +++ b/api/uaa/error_converter.go @@ -53,7 +53,7 @@ func convert(rawHTTPStatusErr RawHTTPStatusError) error { if uaaErrorResponse.Type == "invalid_token" { return InvalidAuthTokenError{Message: uaaErrorResponse.Description} } - if uaaErrorResponse.Type == "unauthorized" { + if uaaErrorResponse.Type == "unauthorized" || uaaErrorResponse.Type == "invalid_client" { if uaaErrorResponse.Description == "Your account has been locked because of too many failed attempts to login." { return AccountLockedError{Message: "Your account has been locked because of too many failed attempts to login."} } diff --git a/api/uaa/error_converter_test.go b/api/uaa/error_converter_test.go index 8d8a03432d..a3fb659690 100644 --- a/api/uaa/error_converter_test.go +++ b/api/uaa/error_converter_test.go @@ -135,6 +135,22 @@ var _ = Describe("Error Wrapper", func() { }) }) + Context("invalid_client with bad credentials", func() { + BeforeEach(func() { + fakeConnectionErr.RawResponse = []byte(`{ + "error": "invalid_client", + "error_description": "Bad credentials" +}`) + fakeConnection.MakeReturns(fakeConnectionErr) + }) + + It("returns a BadCredentialsError", func() { + Expect(fakeConnection.MakeCallCount()).To(Equal(1)) + + Expect(makeErr).To(MatchError(UnauthorizedError{Message: "Bad credentials"})) + }) + }) + Context("unauthorized - too many failed login attempts", func() { BeforeEach(func() { fakeConnectionErr.RawResponse = []byte(`{ diff --git a/integration/v7/isolated/curl_command_test.go b/integration/v7/isolated/curl_command_test.go index 701a46d71b..c65a4b864d 100644 --- a/integration/v7/isolated/curl_command_test.go +++ b/integration/v7/isolated/curl_command_test.go @@ -59,7 +59,6 @@ var _ = Describe("curl command", func() { Eventually(session).Should(Say(`Content-Length: .+`)) Eventually(session).Should(Say(`Content-Type: .+`)) Eventually(session).Should(Say(`Date: .+`)) - Eventually(session).Should(Say(`Server: .+`)) Eventually(session).Should(Say(`X-Content-Type-Options: .+`)) Eventually(session).Should(Say(`X-Vcap-Request-Id: .+`)) } @@ -400,7 +399,6 @@ var _ = Describe("curl command", func() { Expect(contents).To(MatchRegexp(`Content-Length: .+`)) Expect(contents).To(MatchRegexp(`Content-Type: .+`)) Expect(contents).To(MatchRegexp(`Date: .+`)) - Expect(contents).To(MatchRegexp(`Server: .+`)) Expect(contents).To(MatchRegexp(`X-Content-Type-Options: .+`)) Expect(contents).To(MatchRegexp(`X-Vcap-Request-Id: .+`))