From 585988bd83824663895c965966f7e72ca701dd5b Mon Sep 17 00:00:00 2001 From: Elio Bischof Date: Mon, 19 Feb 2024 07:50:37 +0100 Subject: [PATCH] chore(ci): merge test runs (#7388) * chore(ci): merge test runs * docs: improve CONTRIBUTING.md * expect failure * expect failure unit * cleanup * fix: correct binding in integration tests * fix: correct binding in integration tests --------- Co-authored-by: Stefan Benz --- .github/workflows/build.yml | 15 +--- ...ore-integration-test.yml => core-test.yml} | 0 .github/workflows/core-unit-test.yml | 72 ------------------- CONTRIBUTING.md | 44 +++++++----- Makefile | 2 +- internal/api/idp/idp_integration_test.go | 15 +--- internal/integration/client.go | 2 + internal/integration/oidc.go | 15 ++++ 8 files changed, 49 insertions(+), 116 deletions(-) rename .github/workflows/{core-integration-test.yml => core-test.yml} (100%) delete mode 100644 .github/workflows/core-unit-test.yml diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 77eb8e02e..9a6c83d38 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -41,18 +41,9 @@ jobs: console_cache_path: ${{ needs.console.outputs.cache_path }} version: ${{ needs.version.outputs.version }} - core-unit-test: + core-test: needs: core - uses: ./.github/workflows/core-unit-test.yml - with: - go_version: "1.21" - core_cache_key: ${{ needs.core.outputs.cache_key }} - core_cache_path: ${{ needs.core.outputs.cache_path }} - crdb_version: "23.1.13" - - core-integration-test: - needs: core - uses: ./.github/workflows/core-integration-test.yml + uses: ./.github/workflows/core-test.yml with: go_version: "1.21" core_cache_key: ${{ needs.core.outputs.cache_key }} @@ -91,7 +82,7 @@ jobs: issues: write pull-requests: write needs: - [version, core-unit-test, core-integration-test, lint, container, e2e] + [version, core-test, lint, container, e2e] if: ${{ github.event_name == 'workflow_dispatch' }} secrets: GCR_JSON_KEY_BASE64: ${{ secrets.GCR_JSON_KEY_BASE64 }} diff --git a/.github/workflows/core-integration-test.yml b/.github/workflows/core-test.yml similarity index 100% rename from .github/workflows/core-integration-test.yml rename to .github/workflows/core-test.yml diff --git a/.github/workflows/core-unit-test.yml b/.github/workflows/core-unit-test.yml deleted file mode 100644 index 0e69ba2de..000000000 --- a/.github/workflows/core-unit-test.yml +++ /dev/null @@ -1,72 +0,0 @@ -name: Unit test core - -on: - workflow_call: - inputs: - go_version: - required: true - type: string - core_cache_key: - required: true - type: string - core_cache_path: - required: true - type: string - crdb_version: - required: false - type: string - -jobs: - test: - runs-on: - group: zitadel-public - steps: - - - uses: actions/checkout@v3 - - - uses: actions/setup-go@v4 - with: - go-version: ${{ inputs.go_version }} - - - uses: actions/cache/restore@v3 - timeout-minutes: 1 - name: restore core - id: restore-core - with: - path: ${{ inputs.core_cache_path }} - key: ${{ inputs.core_cache_key }} - fail-on-cache-miss: true - - - id: go-cache-path - name: set cache path - run: echo "GO_CACHE_PATH=$(go env GOCACHE)" >> $GITHUB_OUTPUT - - - uses: actions/cache/restore@v3 - id: cache - timeout-minutes: 1 - continue-on-error: true - name: restore previous results - with: - key: unit-test-${{ inputs.core_cache_key }} - restore-keys: | - unit-test-core- - path: ${{ steps.go-cache-path.outputs.GO_CACHE_PATH }} - - - name: test - if: ${{ steps.cache.outputs.cache-hit != 'true' }} - run: export ZITADEL_CRDB_VERSION="${{ inputs.crdb_version }}" && make core_unit_test - - - name: publish coverage - uses: codecov/codecov-action@v3.1.4 - with: - file: profile.cov - name: core-unit-tests - flags: core-unit-tests - - - uses: actions/cache/save@v3 - name: cache results - if: ${{ steps.cache.outputs.cache-hit != 'true' }} - with: - key: unit-test-${{ inputs.core_cache_key }} - path: ${{ steps.go-cache-path.outputs.GO_CACHE_PATH }} - \ No newline at end of file diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 239546acf..a308bacf9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -163,14 +163,33 @@ You can now run and debug the binary in .artifacts/zitadel/zitadel using your fa You can test if ZITADEL does what you expect by using the UI at http://localhost:8080/ui/console. Also, you can verify the data by running `cockroach sql --database zitadel --insecure` and running SQL queries. -As soon as you are ready to battle test your changes, run the end-to-end tests. +#### Run Local Unit Tests -#### Running the tests - -Running the tests with docker doesn't require you to take care of other dependencies than docker and make. +To test the code without dependencies, run the unit tests: ```bash -# Build the production binary (unit tests are executed, too) +make core_unit_test +``` + +#### Run Local Integration Tests + +To test the database-connected gRPC API, run PostgreSQL and CockroachDB, set up a ZITADEL instance and run the tests including integration tests: + +```bash +export INTEGRATION_DB_FLAVOR="postgres" ZITADEL_MASTERKEY="MasterkeyNeedsToHave32Characters" +docker compose -f internal/integration/config/docker-compose.yaml up --pull always --wait ${INTEGRATION_DB_FLAVOR} +make core_integration_test +docker compose -f internal/integration/config/docker-compose.yaml down +``` + +Repeat the above with `INTEGRATION_DB_FLAVOR="postgres"`. + +#### Run Local End-to-End Tests + +To test the whole system, including the console UI and the login UI, run the E2E tests. + +```bash +# Build the production binary make core_build console_build GOOS=linux make compile_pipeline @@ -191,7 +210,7 @@ When you are happy with your changes, you can cleanup your environment. docker compose --file ./e2e/config/host.docker.internal/docker-compose.yaml down ``` -#### Running the tests without docker +#### Run Local End-to-End Tests Against Your Dev Server Console If you also make [changes to the console](#console), you can run the test suite against your locally built backend code and frontend server. But you will have to install the relevant node dependencies. @@ -214,19 +233,6 @@ When you are happy with your changes, you can cleanup your environment. docker compose --file ./e2e/config/host.docker.internal/docker-compose.yaml down ``` -#### Integration tests - -In order to run the integrations tests for the gRPC API, PostgreSQL and CockroachDB must be started and initialized: - -```bash -export INTEGRATION_DB_FLAVOR="postgres" ZITADEL_MASTERKEY="MasterkeyNeedsToHave32Characters" -docker compose -f internal/integration/config/docker-compose.yaml up --pull always --wait ${INTEGRATION_DB_FLAVOR} -make core_integration_test -docker compose -f internal/integration/config/docker-compose.yaml down -``` - -The above can be repeated with `INTEGRATION_DB_FLAVOR="postgres"`. - ### Console By executing the commands from this section, you run everything you need to develop the console locally. diff --git a/Makefile b/Makefile index 8e192a028..188672f69 100644 --- a/Makefile +++ b/Makefile @@ -108,7 +108,7 @@ core_integration_setup: .PHONY: core_integration_test core_integration_test: core_integration_setup - go test -tags=integration -race -p 1 -coverprofile=profile.cov -coverpkg=./internal/...,./cmd/... ./internal/integration ./internal/api/grpc/... ./internal/notification/handlers/... ./internal/api/oidc/... + go test -tags=integration -race -p 1 -coverprofile=profile.cov -coverpkg=./internal/...,./cmd/... ./... .PHONY: console_lint console_lint: diff --git a/internal/api/idp/idp_integration_test.go b/internal/api/idp/idp_integration_test.go index c63fb6808..8ad76a475 100644 --- a/internal/api/idp/idp_integration_test.go +++ b/internal/api/idp/idp_integration_test.go @@ -292,10 +292,7 @@ func TestServer_SAMLACS(t *testing.T) { if tt.args.response != "" { response = tt.args.response } - req := httpPostFormRequest(t, callbackURL, relayState, response) - - //do request to callback URL and check redirect to either success or failure url - location, err := integration.CheckRedirect(req) + location, err := integration.CheckPost(callbackURL, httpPostFormRequest(relayState, response)) if tt.wantErr { require.Error(t, err) } else { @@ -474,15 +471,9 @@ func httpGETRequest(t *testing.T, callbackURL string, relayState, response, sig, return req } -func httpPostFormRequest(t *testing.T, callbackURL, relayState, response string) *http.Request { - body := url.Values{ +func httpPostFormRequest(relayState, response string) url.Values { + return url.Values{ "SAMLResponse": {response}, "RelayState": {relayState}, } - - req, err := http.NewRequest("POST", callbackURL, strings.NewReader(body.Encode())) - assert.NoError(t, err) - req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - req.ParseForm() - return req } diff --git a/internal/integration/client.go b/internal/integration/client.go index 0d53fa657..7a16d3541 100644 --- a/internal/integration/client.go +++ b/internal/integration/client.go @@ -317,6 +317,7 @@ func (s *Tester) AddSAMLRedirectProvider(t *testing.T) string { ctx := authz.WithInstance(context.Background(), s.Instance) id, _, err := s.Server.Commands.AddInstanceSAMLProvider(ctx, command.SAMLProvider{ Name: "saml-idp-redirect", + Binding: "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect", Metadata: []byte("\n \n \n \n \n MIIDBzCCAe+gAwIBAgIJAPr/Mrlc8EGhMA0GCSqGSIb3DQEBBQUAMBoxGDAWBgNVBAMMD3d3dy5leGFtcGxlLmNvbTAeFw0xNTEyMjgxOTE5NDVaFw0yNTEyMjUxOTE5NDVaMBoxGDAWBgNVBAMMD3d3dy5leGFtcGxlLmNvbTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBANDoWzLos4LWxTn8Gyu2lEbl4WcelUbgLN5zYm4ron8Ahs+rvcsu2zkdD/s6jdGJI8WqJKhYK2u61ygnXgAZqC6ggtFPnBpizcDzjgND2g+aucSoUODHt67f0fQuAmupN/zp5MZysJ6IHLJnYLNpfJYk96lRz9ODnO1Mpqtr9PWxm+pz7nzq5F0vRepkgpcRxv6ufQBjlrFytccyEVdXrvFtkjXcnhVVNSR4kHuOOMS6D7pebSJ1mrCmshbD5SX1jXPBKFPAjozYX6PxqLxUx1Y4faFEf4MBBVcInyB4oURNB2s59hEEi2jq9izNE7EbEK6BY5sEhoCPl9m32zE6ljkCAwEAAaNQME4wHQYDVR0OBBYEFB9ZklC1Ork2zl56zg08ei7ss/+iMB8GA1UdIwQYMBaAFB9ZklC1Ork2zl56zg08ei7ss/+iMAwGA1UdEwQFMAMBAf8wDQYJKoZIhvcNAQEFBQADggEBAAVoTSQ5pAirw8OR9FZ1bRSuTDhY9uxzl/OL7lUmsv2cMNeCB3BRZqm3mFt+cwN8GsH6f3uvNONIhgFpTGN5LEcXQz89zJEzB+qaHqmbFpHQl/sx2B8ezNgT/882H2IH00dXESEfy/+1gHg2pxjGnhRBN6el/gSaDiySIMKbilDrffuvxiCfbpPN0NRRiPJhd2ay9KuL/RxQRl1gl9cHaWiouWWba1bSBb2ZPhv2rPMUsFo98ntkGCObDX6Y1SpkqmoTbrsbGFsTG2DLxnvr4GdN1BSr0Uu/KV3adj47WkXVPeMYQti/bQmxQB8tRFhrw80qakTLUzreO96WzlBBMtY=\n \n \n \n \n \n \n MIIDBzCCAe+gAwIBAgIJAPr/Mrlc8EGhMA0GCSqGSIb3DQEBBQUAMBoxGDAWBgNVBAMMD3d3dy5leGFtcGxlLmNvbTAeFw0xNTEyMjgxOTE5NDVaFw0yNTEyMjUxOTE5NDVaMBoxGDAWBgNVBAMMD3d3dy5leGFtcGxlLmNvbTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBANDoWzLos4LWxTn8Gyu2lEbl4WcelUbgLN5zYm4ron8Ahs+rvcsu2zkdD/s6jdGJI8WqJKhYK2u61ygnXgAZqC6ggtFPnBpizcDzjgND2g+aucSoUODHt67f0fQuAmupN/zp5MZysJ6IHLJnYLNpfJYk96lRz9ODnO1Mpqtr9PWxm+pz7nzq5F0vRepkgpcRxv6ufQBjlrFytccyEVdXrvFtkjXcnhVVNSR4kHuOOMS6D7pebSJ1mrCmshbD5SX1jXPBKFPAjozYX6PxqLxUx1Y4faFEf4MBBVcInyB4oURNB2s59hEEi2jq9izNE7EbEK6BY5sEhoCPl9m32zE6ljkCAwEAAaNQME4wHQYDVR0OBBYEFB9ZklC1Ork2zl56zg08ei7ss/+iMB8GA1UdIwQYMBaAFB9ZklC1Ork2zl56zg08ei7ss/+iMAwGA1UdEwQFMAMBAf8wDQYJKoZIhvcNAQEFBQADggEBAAVoTSQ5pAirw8OR9FZ1bRSuTDhY9uxzl/OL7lUmsv2cMNeCB3BRZqm3mFt+cwN8GsH6f3uvNONIhgFpTGN5LEcXQz89zJEzB+qaHqmbFpHQl/sx2B8ezNgT/882H2IH00dXESEfy/+1gHg2pxjGnhRBN6el/gSaDiySIMKbilDrffuvxiCfbpPN0NRRiPJhd2ay9KuL/RxQRl1gl9cHaWiouWWba1bSBb2ZPhv2rPMUsFo98ntkGCObDX6Y1SpkqmoTbrsbGFsTG2DLxnvr4GdN1BSr0Uu/KV3adj47WkXVPeMYQti/bQmxQB8tRFhrw80qakTLUzreO96WzlBBMtY=\n \n \n \n \n \n \n \n urn:oasis:names:tc:SAML:2.0:nameid-format:transient\n \n \n"), IDPOptions: idp.Options{ IsLinkingAllowed: true, @@ -333,6 +334,7 @@ func (s *Tester) AddSAMLPostProvider(t *testing.T) string { ctx := authz.WithInstance(context.Background(), s.Instance) id, _, err := s.Server.Commands.AddInstanceSAMLProvider(ctx, command.SAMLProvider{ Name: "saml-idp-post", + Binding: "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST", Metadata: []byte("\n \n \n \n \n MIIDBzCCAe+gAwIBAgIJAPr/Mrlc8EGhMA0GCSqGSIb3DQEBBQUAMBoxGDAWBgNVBAMMD3d3dy5leGFtcGxlLmNvbTAeFw0xNTEyMjgxOTE5NDVaFw0yNTEyMjUxOTE5NDVaMBoxGDAWBgNVBAMMD3d3dy5leGFtcGxlLmNvbTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBANDoWzLos4LWxTn8Gyu2lEbl4WcelUbgLN5zYm4ron8Ahs+rvcsu2zkdD/s6jdGJI8WqJKhYK2u61ygnXgAZqC6ggtFPnBpizcDzjgND2g+aucSoUODHt67f0fQuAmupN/zp5MZysJ6IHLJnYLNpfJYk96lRz9ODnO1Mpqtr9PWxm+pz7nzq5F0vRepkgpcRxv6ufQBjlrFytccyEVdXrvFtkjXcnhVVNSR4kHuOOMS6D7pebSJ1mrCmshbD5SX1jXPBKFPAjozYX6PxqLxUx1Y4faFEf4MBBVcInyB4oURNB2s59hEEi2jq9izNE7EbEK6BY5sEhoCPl9m32zE6ljkCAwEAAaNQME4wHQYDVR0OBBYEFB9ZklC1Ork2zl56zg08ei7ss/+iMB8GA1UdIwQYMBaAFB9ZklC1Ork2zl56zg08ei7ss/+iMAwGA1UdEwQFMAMBAf8wDQYJKoZIhvcNAQEFBQADggEBAAVoTSQ5pAirw8OR9FZ1bRSuTDhY9uxzl/OL7lUmsv2cMNeCB3BRZqm3mFt+cwN8GsH6f3uvNONIhgFpTGN5LEcXQz89zJEzB+qaHqmbFpHQl/sx2B8ezNgT/882H2IH00dXESEfy/+1gHg2pxjGnhRBN6el/gSaDiySIMKbilDrffuvxiCfbpPN0NRRiPJhd2ay9KuL/RxQRl1gl9cHaWiouWWba1bSBb2ZPhv2rPMUsFo98ntkGCObDX6Y1SpkqmoTbrsbGFsTG2DLxnvr4GdN1BSr0Uu/KV3adj47WkXVPeMYQti/bQmxQB8tRFhrw80qakTLUzreO96WzlBBMtY=\n \n \n \n \n \n \n MIIDBzCCAe+gAwIBAgIJAPr/Mrlc8EGhMA0GCSqGSIb3DQEBBQUAMBoxGDAWBgNVBAMMD3d3dy5leGFtcGxlLmNvbTAeFw0xNTEyMjgxOTE5NDVaFw0yNTEyMjUxOTE5NDVaMBoxGDAWBgNVBAMMD3d3dy5leGFtcGxlLmNvbTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBANDoWzLos4LWxTn8Gyu2lEbl4WcelUbgLN5zYm4ron8Ahs+rvcsu2zkdD/s6jdGJI8WqJKhYK2u61ygnXgAZqC6ggtFPnBpizcDzjgND2g+aucSoUODHt67f0fQuAmupN/zp5MZysJ6IHLJnYLNpfJYk96lRz9ODnO1Mpqtr9PWxm+pz7nzq5F0vRepkgpcRxv6ufQBjlrFytccyEVdXrvFtkjXcnhVVNSR4kHuOOMS6D7pebSJ1mrCmshbD5SX1jXPBKFPAjozYX6PxqLxUx1Y4faFEf4MBBVcInyB4oURNB2s59hEEi2jq9izNE7EbEK6BY5sEhoCPl9m32zE6ljkCAwEAAaNQME4wHQYDVR0OBBYEFB9ZklC1Ork2zl56zg08ei7ss/+iMB8GA1UdIwQYMBaAFB9ZklC1Ork2zl56zg08ei7ss/+iMAwGA1UdEwQFMAMBAf8wDQYJKoZIhvcNAQEFBQADggEBAAVoTSQ5pAirw8OR9FZ1bRSuTDhY9uxzl/OL7lUmsv2cMNeCB3BRZqm3mFt+cwN8GsH6f3uvNONIhgFpTGN5LEcXQz89zJEzB+qaHqmbFpHQl/sx2B8ezNgT/882H2IH00dXESEfy/+1gHg2pxjGnhRBN6el/gSaDiySIMKbilDrffuvxiCfbpPN0NRRiPJhd2ay9KuL/RxQRl1gl9cHaWiouWWba1bSBb2ZPhv2rPMUsFo98ntkGCObDX6Y1SpkqmoTbrsbGFsTG2DLxnvr4GdN1BSr0Uu/KV3adj47WkXVPeMYQti/bQmxQB8tRFhrw80qakTLUzreO96WzlBBMtY=\n \n \n \n \n \n \n \n urn:oasis:names:tc:SAML:2.0:nameid-format:transient\n \n \n"), IDPOptions: idp.Options{ IsLinkingAllowed: true, diff --git a/internal/integration/oidc.go b/internal/integration/oidc.go index 16c4c90ae..6421758ad 100644 --- a/internal/integration/oidc.go +++ b/internal/integration/oidc.go @@ -238,6 +238,21 @@ func GetRequest(url string, headers map[string]string) (*http.Request, error) { return req, nil } +func CheckPost(url string, values url.Values) (*url.URL, error) { + client := &http.Client{ + CheckRedirect: func(req *http.Request, via []*http.Request) error { + return http.ErrUseLastResponse + }, + } + resp, err := client.PostForm(url, values) + if err != nil { + return nil, err + } + defer resp.Body.Close() + + return resp.Location() +} + func CheckRedirect(req *http.Request) (*url.URL, error) { client := &http.Client{ CheckRedirect: func(req *http.Request, via []*http.Request) error {