diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 06be1797a1..32de6b633c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,3 +1,4 @@ + # Contributing to ZITADEL ## Introduction @@ -34,7 +35,11 @@ Follow [@zitadel](https://twitter.com/zitadel) on twitter We strongly recommend to [talk to us](https://zitadel.com/contact) before you start contributing to streamline our and your work. -We accept contributions through pull requests. You need a github account for that. If you are unfamiliar with git have a look at Github's documentation on [creating forks](https://help.github.com/articles/fork-a-repo) and [creating pull requests](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-a-pull-request-from-a-fork). Please draft the pull request as soon as possible. Go through the following checklist before you submit the final pull request: +We accept contributions through pull requests. +You need a github account for that. +If you are unfamiliar with git have a look at Github's documentation on [creating forks](https://help.github.com/articles/fork-a-repo) and [creating pull requests](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-a-pull-request-from-a-fork). +Please draft the pull request as soon as possible. +Go through the following checklist before you submit the final pull request: ### Submit a pull request (PR) @@ -61,7 +66,8 @@ We accept contributions through pull requests. You need a github account for tha ### Review a pull request -The reviewers will provide you feedback and approve your changes as soon as they are satisfied. If we ask you for changes in the code, you can follow the [GitHub Guide](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/incorporating-feedback-in-your-pull-request) to incorporate feedback in your pull request. +The reviewers will provide you feedback and approve your changes as soon as they are satisfied. +If we ask you for changes in the code, you can follow the [GitHub Guide](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/incorporating-feedback-in-your-pull-request) to incorporate feedback in your pull request. @@ -88,6 +94,16 @@ This is optional to indicate which component is affected. In doubt, leave blank Provide a brief description of the change. +### Quality assurance + +Please make sure you cover your changes with tests before marking a Pull Request as ready for review: + +- [ ] Integration tests against the gRPC server ensure that one or multiple API calls that belong together return the expected results. +- [ ] Integration tests against the gRPC server ensure that probable good and bad read and write permissions are tested. +- [ ] Integration tests against the gRPC server ensure that the API is easily usable despite eventual consistency. +- [ ] Integration tests against the gRPC server ensure that all probable login and registration flows are covered." +- [ ] Integration tests ensure that certain commands send expected notifications. + ## Contribute The code consists of the following parts: diff --git a/cmd/defaults.yaml b/cmd/defaults.yaml index bf8d093f3e..2a283f44f2 100644 --- a/cmd/defaults.yaml +++ b/cmd/defaults.yaml @@ -15,16 +15,16 @@ Tracing: MetricPrefix: zitadel Telemetry: - # As long as Enabled is true, usage data that is marked as due is also tried to be sent to the configured Telemetry.Endpoints. - # Data is marked as due, even if Enabled is false. - # This means that switching this to true makes ZITADEL try to send past data that was marked to send. + # As long as Enabled is true, ZITADEL tries to send usage data to the configured Telemetry.Endpoints. + # Data is projected by ZITADEL even if Enabled is false. + # This means that switching this to true makes ZITADEL try to send past data. Enabled: false - # Push telemetry data to all these endpoints at least once. + # Push telemetry data to all these endpoints at least once using an HTTP POST request. # If one endpoint returns an unsuccessful response code or times out, # ZITADEL retries to push the data point to all configured endpoints until it succeeds. # Configure delivery guarantees and intervals in the section Projections.Customizations.Telemetry # The endpoints can be reconfigured at runtime. - # Three redirects are followed. + # Ten redirects are followed. # If you change this configuration at runtime, remaining data that is not successfully delivered to the old endpoints is sent to the new endpoints. Endpoints: # Include https://zitadel.com/usage if you want to help the ZITADEL maintainers improve the product's usability by giving them some insights. @@ -207,7 +207,7 @@ Projections: HandleActiveInstances: 360h # As sending telemetry data doesn't result in database statements, retries don't have any effects MaxFailureCount: 0 - # Telemetry data synchronization is not time critical. Setting RequeueEvery every 55 minutes doesn't annoy the database too much. + # Telemetry data synchronization is not time critical. Setting RequeueEvery to 55 minutes doesn't annoy the database too much. RequeueEvery: 3300s Auth: diff --git a/internal/notification/handlers/handlers_integration_test.go b/internal/notification/handlers/handlers_integration_test.go index 3bf9c9f87f..8a7a0b085d 100644 --- a/internal/notification/handlers/handlers_integration_test.go +++ b/internal/notification/handlers/handlers_integration_test.go @@ -16,22 +16,20 @@ import ( ) var ( - Tester *integration.Tester - SystemUserCTX context.Context - SystemClient system.SystemServiceClient - PrimaryDomain, InstanceID string - IAMOwnerCtx context.Context - MgmtClient management.ManagementServiceClient + CTX context.Context + Tester *integration.Tester + SystemClient system.SystemServiceClient + MgmtClient management.ManagementServiceClient ) func TestMain(m *testing.M) { os.Exit(func() int { ctx, _, cancel := integration.Contexts(5 * time.Minute) + CTX = ctx defer cancel() os.Setenv("INTEGRATION_DB_FLAVOR", "postgres") os.Setenv("ZITADEL_MASTERKEY", "MasterkeyNeedsToHave32Characters") Tester = integration.NewTester(ctx) - PrimaryDomain, InstanceID, SystemUserCTX, IAMOwnerCtx = Tester.UseIsolatedInstance(ctx) MgmtClient = Tester.Client.Mgmt SystemClient = Tester.Client.System defer Tester.Done() diff --git a/internal/notification/handlers/telemetry_pusher_integration_test.go b/internal/notification/handlers/telemetry_pusher_integration_test.go index e30f3b3a6c..1f4a314f1d 100644 --- a/internal/notification/handlers/telemetry_pusher_integration_test.go +++ b/internal/notification/handlers/telemetry_pusher_integration_test.go @@ -16,9 +16,10 @@ import ( "github.com/zitadel/zitadel/pkg/grpc/system" ) -func TestServer_TelemetryPusher(t *testing.T) { +func TestServer_TelemetryPushMilestones(t *testing.T) { + primaryDomain, instanceID, systemUserCTX, iamOwnerCtx := Tester.UseIsolatedInstance(CTX) bodies := make(chan []byte, 0) - t.Log("testing against instance with primary domain", PrimaryDomain) + t.Log("testing against instance with primary domain", primaryDomain) mockServer := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { body, err := io.ReadAll(r.Body) if err != nil { @@ -34,26 +35,26 @@ func TestServer_TelemetryPusher(t *testing.T) { mockServer.Listener = listener mockServer.Start() t.Cleanup(mockServer.Close) - awaitMilestone(t, bodies, "InstanceCreated") - project, err := MgmtClient.AddProject(IAMOwnerCtx, &management.AddProjectRequest{Name: "integration"}) + awaitMilestone(t, bodies, primaryDomain, "InstanceCreated") + project, err := MgmtClient.AddProject(iamOwnerCtx, &management.AddProjectRequest{Name: "integration"}) if err != nil { t.Fatal(err) } - awaitMilestone(t, bodies, "ProjectCreated") - if _, err = MgmtClient.AddOIDCApp(IAMOwnerCtx, &management.AddOIDCAppRequest{ + awaitMilestone(t, bodies, primaryDomain, "ProjectCreated") + if _, err = MgmtClient.AddOIDCApp(iamOwnerCtx, &management.AddOIDCAppRequest{ ProjectId: project.GetId(), Name: "integration", }); err != nil { t.Fatal(err) } - awaitMilestone(t, bodies, "ApplicationCreated") - if _, err = SystemClient.RemoveInstance(SystemUserCTX, &system.RemoveInstanceRequest{InstanceId: InstanceID}); err != nil { + awaitMilestone(t, bodies, primaryDomain, "ApplicationCreated") + if _, err = SystemClient.RemoveInstance(systemUserCTX, &system.RemoveInstanceRequest{InstanceId: instanceID}); err != nil { t.Fatal(err) } - awaitMilestone(t, bodies, "InstanceDeleted") + awaitMilestone(t, bodies, primaryDomain, "InstanceDeleted") } -func awaitMilestone(t *testing.T, bodies chan []byte, expectMilestoneType string) { +func awaitMilestone(t *testing.T, bodies chan []byte, primaryDomain, expectMilestoneType string) { for { select { case body := <-bodies: @@ -69,11 +70,11 @@ func awaitMilestone(t *testing.T, bodies chan []byte, expectMilestoneType string if err := json.Unmarshal(body, &milestone); err != nil { t.Error(err) } - if milestone.Type == expectMilestoneType && milestone.PrimaryDomain == PrimaryDomain { + if milestone.Type == expectMilestoneType && milestone.PrimaryDomain == primaryDomain { return } case <-time.After(60 * time.Second): - t.Fatalf("timed out waiting for milestone") + t.Fatalf("timed out waiting for milestone %s in domain %s", expectMilestoneType, primaryDomain) } } }