Fix pr issues (cloudfoundry#2968)

* 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 cloudfoundry/capi-release#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 cloudfoundry/uaa#2545 Signed-off-by: João Pereira <[email protected]>
sap-contributions · Jun 20, 2024 · 36abb81 · 36abb81
1 parent e48120d
commit 36abb81
Show file tree

Hide file tree

Showing 6 changed files with 45 additions and 51 deletions.
diff --git 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/[email protected]
-      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/[email protected]
-      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
@@ -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,53 +20,45 @@ 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
       run-with-client-creds: false
       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
@@ -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
@@ -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
@@ -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
@@ -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: .+`))