From a52de05758e5a449bc526393ae832ea12d6e9fb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Pereira?= Date: Fri, 28 Jun 2024 11:03:41 -0500 Subject: [PATCH] [v7] Fix pr issues (#2974) 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 * Revert min-capi tests introduction * Incorrect merge of cherry-pick Signed-off-by: João Pereira --- .../workflows/tests-integration-reusable.yml | 103 +++++------------- .github/workflows/tests-integration.yml | 54 +++++---- .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 | 5 +- 6 files changed, 88 insertions(+), 102 deletions(-) diff --git a/.github/workflows/tests-integration-reusable.yml b/.github/workflows/tests-integration-reusable.yml index 6239c51dc38..cff651d035c 100644 --- a/.github/workflows/tests-integration-reusable.yml +++ b/.github/workflows/tests-integration-reusable.yml @@ -22,7 +22,12 @@ on: name: required: true type: string - + pool-name: + type: string + default: ${{ vars.SHEPHERD_POOL_NAME }} + pool-namespace: + type: string + default: 'official' jobs: run-integration-tests: defaults: @@ -31,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() - 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: @@ -86,12 +80,14 @@ jobs: name: Claim Environment env: account_token: ${{ secrets.SHEPHERD_SERVICE_ACCOUNT_TOKEN }} - pool_name: ${{ vars.SHEPHERD_POOL_NAME }} + pool_name: ${{ inputs.pool-name }} + pool_namespace: ${{ inputs.pool-namespace }} run: | shepherd login service-account ${account_token} - lease_id=$(shepherd create lease --duration 8h --pool ${pool_name} --pool-namespace official --namespace tas-devex --json | jq -r .id) - # Give somtime for the lease to complete. Shepherd may take upto an 3 hours to create an env + 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 @@ -112,7 +108,7 @@ jobs: env_name=$(jq -r .name metadata.json) cat metadata.json | jq -r '.name' echo "lease-id=$lease_id" >> "${GITHUB_OUTPUT}" - + - name: Install Tools run: | if [[ ${{ inputs.os }} =~ "windows" ]] @@ -146,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 @@ -161,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 @@ -171,57 +165,24 @@ jobs: - name: Deploy MIN CAPI with Isolation Segment and OIDC Provider if: ${{ inputs.capi-version != 'edge' }} run: | - # TODO: Make this actually deploy min capi - # Creates vars files - mkdir vars-files - echo "cs = ${{ secrets.CLIENT_SECRET }}" - cat << EOF > vars-files/vars.yml - client-secret: ${{ secrets.CLIENT_SECRET }} - EOF - - # Copy Ops files - mkdir ops-files - cp cf-deployment/operations/test/add-persistent-isolation-segment-diego-cell.yml ops-files/ - cp cli-ci/ci/infrastructure/operations/add-oidc-provider.yml ops-files/ - cp cli-ci/ci/infrastructure/operations/add-uaa-client-credentials.yml ops-files/ - cp cli-ci/ci/infrastructure/operations/diego-cell-instances.yml ops-files/ - cp cli-ci/ci/infrastructure/operations/use-latest-ruby-buildpack.yml ops-files/ - - # Deletes CF-D env_name=$(jq -r .name metadata.json) jq -r .bosh.jumpbox_private_key metadata.json > /tmp/${env_name}.priv eval "$(bbl print-env --metadata-file metadata.json)" - bosh -d cf delete-deployment -n - # Deploy CF-D - mkdir toolsmiths-env - cp metadata.json toolsmiths-env/metadata - cat metadata.json | jq -r .name > toolsmiths-env/name - export VARS_FILES="vars.yml" - export MANIFEST_FILE="cf-deployment.yml" - export SYSTEM_DOMAIN="" - export REGENERATE_CREDENTIALS=false - export DEPLOY_WITH_UPTIME_MEASUREMENTS=false - export MEASURE_SYSLOG_AVAILABILITY=false - export TCP_DOMAIN="" - export AVAILABLE_PORT="" - export FAIL_ON_DOWNTIME=false - export APP_PUSHABILITY_THRESHOLD=0 - export HTTP_AVAILABILITY_THRESHOLD=0 - export RECENT_LOGS_THRESHOLD=0 - export STREAMING_LOGS_THRESHOLD=0 - export APP_SYSLOG_AVAILABILITY_THRESHOLD=0 - export USE_SINGLE_APP_INSTANCE=false - export BOSH_DEPLOY_ARGS="" - export BOSH_LITE=false - export BBL_JSON_CONFIG="" - export SKIP_STEMCELL_UPLOAD=false - export OPS_FILES="add-persistent-isolation-segment-diego-cell.yml \ - add-uaa-client-credentials.yml \ - diego-cell-instances.yml \ - add-oidc-provider.yml \ - use-latest-ruby-buildpack.yml" - ./cf-deployment-concourse-tasks/bosh-deploy/task + # deploy + bosh -d cf manifest > /tmp/manifest.yml + bosh interpolate /tmp/manifest.yml \ + -o cf-deployment/operations/test/add-persistent-isolation-segment-diego-cell.yml \ + -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 \ + -o cli-ci/ci/infrastructure/operations/use-latest-ruby-buildpack.yml \ + -v client-secret="${{ secrets.CLIENT_SECRET }}" \ + > ./director.yml + + bosh -d cf deploy director.yml -n + echo "Deployed CAPI version:" + bosh -d cf releases | grep capi - name: Set Up Go uses: actions/setup-go@v5 @@ -294,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() - 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 b49c93963c7..db0fc752224 100644 --- a/.github/workflows/tests-integration.yml +++ b/.github/workflows/tests-integration.yml @@ -3,17 +3,27 @@ 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: - workflow_run: - workflows: - - "Tests" - types: - - completed - -jobs: + inputs: + workflow: + description: Tests to run + required: true + type: choice + options: + - run-integration-tests-cf-env + - run-integration-tests-cf-env-with-client-creds + # - run-integration-tests-cf-env-with-min-capi + # - run-integration-windows + # - run-integration-windows-client-credentials +jobs: run-integration-tests-cf-env: name: Integration tests - if: ${{ github.event_name == 'workflow_dispatch' || 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 @@ -24,7 +34,7 @@ jobs: run-integration-tests-cf-env-with-client-creds: name: client creds - if: ${{ github.event_name == 'workflow_dispatch' || 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 @@ -33,20 +43,22 @@ jobs: name: Integration client creds secrets: inherit - # run-integration-tests-cf-env-with-min-capi: - # name: MIN CAPI - # # if: ${{ github.event_name == 'workflow_dispatch' || github.event.workflow_run.conclusion == 'success' }} - # uses: ./.github/workflows/tests-integration-reusable.yml - # with: - # capi-version: min - # run-with-client-creds: false - # os: ubuntu-latest - # name: Integration MIN CAPI - # secrets: inherit + #run-integration-tests-cf-env-with-min-capi: + # name: MIN CAPI + # 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 + # run-with-client-creds: false + # os: ubuntu-latest + # name: Integration MIN CAPI + # pool-name: cfd_16_11_0 + # pool-namespace: tas-devex + # secrets: inherit #run-integration-windows: # name: Windows - # if: ${{ github.event_name == 'workflow_dispatch' || 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 @@ -57,7 +69,7 @@ jobs: #run-integration-windows-client-credentials: # name: Windows with client credentials - # if: ${{ github.event_name == 'workflow_dispatch' || 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 51b23b8132c..0301a89b4ee 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 ebbe950caee..c5ecb299214 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 8d8a03432d9..a3fb6596904 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 5cc7f01d5bd..cef5f0d4a5d 100644 --- a/integration/v7/isolated/curl_command_test.go +++ b/integration/v7/isolated/curl_command_test.go @@ -8,7 +8,6 @@ import ( "strings" . "code.cloudfoundry.org/cli/cf/util/testhelpers/matchers" - "code.cloudfoundry.org/cli/integration/helpers" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -57,11 +56,9 @@ var _ = Describe("curl command", func() { var ExpectResponseHeaders = func(session *Session) { Eventually(session).Should(Say("HTTP/1.1 200 OK")) - Eventually(session).Should(Say(`Connection: .+`)) 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: .+`)) } @@ -386,7 +383,7 @@ var _ = Describe("curl command", func() { }) When("--output is passed with a file name", func() { - It("writes the response body to the file but the other output to stdout", func() { + It("writes the response headers and body to the file", func() { outFile, err := os.CreateTemp("", "output*.json") Expect(err).ToNot(HaveOccurred()) session := helpers.CF("curl", "/v2/apps", "-i", "--output", outFile.Name())