From 66bac709c863cc6af255330bf7d6d23407be9407 Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Mon, 17 Jun 2024 11:31:18 -0400 Subject: [PATCH 1/4] fix: add external auth configs to telemetry --- cli/server.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/cli/server.go b/cli/server.go index 11c3ec50ba833..9a0f51c6e6427 100644 --- a/cli/server.go +++ b/cli/server.go @@ -797,9 +797,7 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd. if vals.Telemetry.Enable { gitAuth := make([]telemetry.GitAuth, 0) - // TODO: - gitAuthConfigs := make([]codersdk.ExternalAuthConfig, 0) - for _, cfg := range gitAuthConfigs { + for _, cfg := range externalAuthConfigs { gitAuth = append(gitAuth, telemetry.GitAuth{ Type: cfg.Type, }) From a4e3e09ae84928e40bbd91485cb0b2c7e5122453 Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Mon, 17 Jun 2024 14:02:09 -0400 Subject: [PATCH 2/4] Refactor telemetry to send the entire config --- cli/server.go | 31 +++---- coderd/telemetry/telemetry.go | 130 ++++++++++------------------- coderd/telemetry/telemetry_test.go | 11 --- codersdk/deployment.go | 2 +- 4 files changed, 54 insertions(+), 120 deletions(-) diff --git a/cli/server.go b/cli/server.go index 9a0f51c6e6427..79d2b132ad6e3 100644 --- a/cli/server.go +++ b/cli/server.go @@ -796,29 +796,18 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd. cliui.Infof(inv.Stdout, "\n==> Logs will stream in below (press ctrl+c to gracefully exit):") if vals.Telemetry.Enable { - gitAuth := make([]telemetry.GitAuth, 0) - for _, cfg := range externalAuthConfigs { - gitAuth = append(gitAuth, telemetry.GitAuth{ - Type: cfg.Type, - }) + vals, err := vals.WithoutSecrets() + if err != nil { + return xerrors.Errorf("remove secrets from deployment values: %w", err) } - options.Telemetry, err = telemetry.New(telemetry.Options{ - BuiltinPostgres: builtinPostgres, - DeploymentID: deploymentID, - Database: options.Database, - Logger: logger.Named("telemetry"), - URL: vals.Telemetry.URL.Value(), - Wildcard: vals.WildcardAccessURL.String() != "", - DERPServerRelayURL: vals.DERP.Server.RelayURL.String(), - GitAuth: gitAuth, - GitHubOAuth: vals.OAuth2.Github.ClientID != "", - OIDCAuth: vals.OIDC.ClientID != "", - OIDCIssuerURL: vals.OIDC.IssuerURL.String(), - Prometheus: vals.Prometheus.Enable.Value(), - STUN: len(vals.DERP.Server.STUNAddresses) != 0, - Tunnel: tunnel != nil, - Experiments: vals.Experiments.Value(), + BuiltinPostgres: builtinPostgres, + DeploymentID: deploymentID, + Database: options.Database, + Logger: logger.Named("telemetry"), + URL: vals.Telemetry.URL.Value(), + Tunnel: tunnel != nil, + DeploymentConfig: vals, ParseLicenseJWT: func(lic *telemetry.License) error { // This will be nil when running in AGPL-only mode. if options.ParseLicenseClaims == nil { diff --git a/coderd/telemetry/telemetry.go b/coderd/telemetry/telemetry.go index 36292179da478..1b9489db3af8f 100644 --- a/coderd/telemetry/telemetry.go +++ b/coderd/telemetry/telemetry.go @@ -41,20 +41,13 @@ type Options struct { // URL is an endpoint to direct telemetry towards! URL *url.URL - BuiltinPostgres bool - DeploymentID string - GitHubOAuth bool - OIDCAuth bool - OIDCIssuerURL string - Wildcard bool - DERPServerRelayURL string - GitAuth []GitAuth - Prometheus bool - STUN bool - SnapshotFrequency time.Duration - Tunnel bool - ParseLicenseJWT func(lic *License) error - Experiments []string + DeploymentID string + DeploymentConfig *codersdk.DeploymentValues + BuiltinPostgres bool + Tunnel bool + + SnapshotFrequency time.Duration + ParseLicenseJWT func(lic *License) error } // New constructs a reporter for telemetry data. @@ -242,31 +235,24 @@ func (r *remoteReporter) deployment() error { } data, err := json.Marshal(&Deployment{ - ID: r.options.DeploymentID, - Architecture: sysInfo.Architecture, - BuiltinPostgres: r.options.BuiltinPostgres, - Containerized: containerized, - Wildcard: r.options.Wildcard, - DERPServerRelayURL: r.options.DERPServerRelayURL, - GitAuth: r.options.GitAuth, - Kubernetes: os.Getenv("KUBERNETES_SERVICE_HOST") != "", - GitHubOAuth: r.options.GitHubOAuth, - OIDCAuth: r.options.OIDCAuth, - OIDCIssuerURL: r.options.OIDCIssuerURL, - Prometheus: r.options.Prometheus, - InstallSource: installSource, - STUN: r.options.STUN, - Tunnel: r.options.Tunnel, - OSType: sysInfo.OS.Type, - OSFamily: sysInfo.OS.Family, - OSPlatform: sysInfo.OS.Platform, - OSName: sysInfo.OS.Name, - OSVersion: sysInfo.OS.Version, - CPUCores: runtime.NumCPU(), - MemoryTotal: mem.Total, - MachineID: sysInfo.UniqueID, - StartedAt: r.startedAt, - ShutdownAt: r.shutdownAt, + ID: r.options.DeploymentID, + Architecture: sysInfo.Architecture, + BuiltinPostgres: r.options.BuiltinPostgres, + Containerized: containerized, + Config: r.options.DeploymentConfig, + Kubernetes: os.Getenv("KUBERNETES_SERVICE_HOST") != "", + InstallSource: installSource, + Tunnel: r.options.Tunnel, + OSType: sysInfo.OS.Type, + OSFamily: sysInfo.OS.Family, + OSPlatform: sysInfo.OS.Platform, + OSName: sysInfo.OS.Name, + OSVersion: sysInfo.OS.Version, + CPUCores: runtime.NumCPU(), + MemoryTotal: mem.Total, + MachineID: sysInfo.UniqueID, + StartedAt: r.startedAt, + ShutdownAt: r.shutdownAt, }) if err != nil { return xerrors.Errorf("marshal deployment: %w", err) @@ -481,10 +467,6 @@ func (r *remoteReporter) createSnapshot() (*Snapshot, error) { } return nil }) - eg.Go(func() error { - snapshot.Experiments = ConvertExperiments(r.options.Experiments) - return nil - }) err := eg.Wait() if err != nil { @@ -745,16 +727,6 @@ func ConvertExternalProvisioner(id uuid.UUID, tags map[string]string, provisione } } -func ConvertExperiments(experiments []string) []Experiment { - var out []Experiment - - for _, exp := range experiments { - out = append(out, Experiment{Name: exp}) - } - - return out -} - // Snapshot represents a point-in-time anonymized database dump. // Data is aggregated by latest on the server-side, so partial data // can be sent without issue. @@ -777,40 +749,28 @@ type Snapshot struct { WorkspaceResourceMetadata []WorkspaceResourceMetadata `json:"workspace_resource_metadata"` WorkspaceResources []WorkspaceResource `json:"workspace_resources"` Workspaces []Workspace `json:"workspaces"` - Experiments []Experiment `json:"experiments"` } // Deployment contains information about the host running Coder. type Deployment struct { - ID string `json:"id"` - Architecture string `json:"architecture"` - BuiltinPostgres bool `json:"builtin_postgres"` - Containerized bool `json:"containerized"` - Kubernetes bool `json:"kubernetes"` - Tunnel bool `json:"tunnel"` - Wildcard bool `json:"wildcard"` - DERPServerRelayURL string `json:"derp_server_relay_url"` - GitAuth []GitAuth `json:"git_auth"` - GitHubOAuth bool `json:"github_oauth"` - OIDCAuth bool `json:"oidc_auth"` - OIDCIssuerURL string `json:"oidc_issuer_url"` - Prometheus bool `json:"prometheus"` - InstallSource string `json:"install_source"` - STUN bool `json:"stun"` - OSType string `json:"os_type"` - OSFamily string `json:"os_family"` - OSPlatform string `json:"os_platform"` - OSName string `json:"os_name"` - OSVersion string `json:"os_version"` - CPUCores int `json:"cpu_cores"` - MemoryTotal uint64 `json:"memory_total"` - MachineID string `json:"machine_id"` - StartedAt time.Time `json:"started_at"` - ShutdownAt *time.Time `json:"shutdown_at"` -} - -type GitAuth struct { - Type string `json:"type"` + ID string `json:"id"` + Architecture string `json:"architecture"` + BuiltinPostgres bool `json:"builtin_postgres"` + Containerized bool `json:"containerized"` + Kubernetes bool `json:"kubernetes"` + Config *codersdk.DeploymentValues `json:"config"` + Tunnel bool `json:"tunnel"` + InstallSource string `json:"install_source"` + OSType string `json:"os_type"` + OSFamily string `json:"os_family"` + OSPlatform string `json:"os_platform"` + OSName string `json:"os_name"` + OSVersion string `json:"os_version"` + CPUCores int `json:"cpu_cores"` + MemoryTotal uint64 `json:"memory_total"` + MachineID string `json:"machine_id"` + StartedAt time.Time `json:"started_at"` + ShutdownAt *time.Time `json:"shutdown_at"` } type APIKey struct { @@ -985,10 +945,6 @@ type ExternalProvisioner struct { ShutdownAt *time.Time `json:"shutdown_at"` } -type Experiment struct { - Name string `json:"name"` -} - type noopReporter struct{} func (*noopReporter) Report(_ *Snapshot) {} diff --git a/coderd/telemetry/telemetry_test.go b/coderd/telemetry/telemetry_test.go index 4661a4f8f21bf..fa8650de3f3d5 100644 --- a/coderd/telemetry/telemetry_test.go +++ b/coderd/telemetry/telemetry_test.go @@ -114,17 +114,6 @@ func TestTelemetry(t *testing.T) { require.Len(t, snapshot.Users, 1) require.Equal(t, snapshot.Users[0].EmailHashed, "bb44bf07cf9a2db0554bba63a03d822c927deae77df101874496df5a6a3e896d@coder.com") }) - t.Run("Experiments", func(t *testing.T) { - t.Parallel() - - const expName = "my-experiment" - exps := []string{expName} - _, snapshot := collectSnapshot(t, dbmem.New(), func(opts telemetry.Options) telemetry.Options { - opts.Experiments = exps - return opts - }) - require.Equal(t, []telemetry.Experiment{{Name: expName}}, snapshot.Experiments) - }) } // nolint:paralleltest diff --git a/codersdk/deployment.go b/codersdk/deployment.go index b67964d6a985c..ff35d67bacbb4 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -393,7 +393,7 @@ type ExternalAuthConfig struct { AppInstallationsURL string `json:"app_installations_url" yaml:"app_installations_url"` NoRefresh bool `json:"no_refresh" yaml:"no_refresh"` Scopes []string `json:"scopes" yaml:"scopes"` - ExtraTokenKeys []string `json:"extra_token_keys" yaml:"extra_token_keys"` + ExtraTokenKeys []string `json:"-" yaml:"extra_token_keys"` DeviceFlow bool `json:"device_flow" yaml:"device_flow"` DeviceCodeURL string `json:"device_code_url" yaml:"device_code_url"` // Regex allows API requesters to match an auth config by From 78266e5d3a3c3498bcaea1539eb66c35a83877e2 Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Mon, 17 Jun 2024 18:26:17 +0000 Subject: [PATCH 3/4] gen --- coderd/apidoc/docs.go | 6 ------ coderd/apidoc/swagger.json | 6 ------ docs/api/general.md | 1 - docs/api/schemas.md | 5 ----- site/src/api/typesGenerated.ts | 1 - 5 files changed, 19 deletions(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 43ab78ffc1eeb..508b11e83083c 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -9349,12 +9349,6 @@ const docTemplate = `{ "description": "DisplayName is shown in the UI to identify the auth config.", "type": "string" }, - "extra_token_keys": { - "type": "array", - "items": { - "type": "string" - } - }, "id": { "description": "ID is a unique identifier for the auth config.\nIt defaults to ` + "`" + `type` + "`" + ` when not provided.", "type": "string" diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 8871bcf89e502..b114a85affced 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -8379,12 +8379,6 @@ "description": "DisplayName is shown in the UI to identify the auth config.", "type": "string" }, - "extra_token_keys": { - "type": "array", - "items": { - "type": "string" - } - }, "id": { "description": "ID is a unique identifier for the auth config.\nIt defaults to `type` when not provided.", "type": "string" diff --git a/docs/api/general.md b/docs/api/general.md index 84424331cf488..a92742ce0a707 100644 --- a/docs/api/general.md +++ b/docs/api/general.md @@ -227,7 +227,6 @@ curl -X GET http://coder-server:8080/api/v2/deployment/config \ "device_flow": true, "display_icon": "string", "display_name": "string", - "extra_token_keys": ["string"], "id": "string", "no_refresh": true, "regex": "string", diff --git a/docs/api/schemas.md b/docs/api/schemas.md index bc3d0440efb9c..cd691e1ad71fc 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -1646,7 +1646,6 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "device_flow": true, "display_icon": "string", "display_name": "string", - "extra_token_keys": ["string"], "id": "string", "no_refresh": true, "regex": "string", @@ -2020,7 +2019,6 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "device_flow": true, "display_icon": "string", "display_name": "string", - "extra_token_keys": ["string"], "id": "string", "no_refresh": true, "regex": "string", @@ -2441,7 +2439,6 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "device_flow": true, "display_icon": "string", "display_name": "string", - "extra_token_keys": ["string"], "id": "string", "no_refresh": true, "regex": "string", @@ -2464,7 +2461,6 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o | `device_flow` | boolean | false | | | | `display_icon` | string | false | | Display icon is a URL to an icon to display in the UI. | | `display_name` | string | false | | Display name is shown in the UI to identify the auth config. | -| `extra_token_keys` | array of string | false | | | | `id` | string | false | | ID is a unique identifier for the auth config. It defaults to `type` when not provided. | | `no_refresh` | boolean | false | | | | `regex` | string | false | | Regex allows API requesters to match an auth config by a string (e.g. coder.com) instead of by it's type. | @@ -8546,7 +8542,6 @@ _None_ "device_flow": true, "display_icon": "string", "display_name": "string", - "extra_token_keys": ["string"], "id": "string", "no_refresh": true, "regex": "string", diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index c5b67b387b8b9..0d9147c912e9e 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -509,7 +509,6 @@ export interface ExternalAuthConfig { readonly app_installations_url: string; readonly no_refresh: boolean; readonly scopes: readonly string[]; - readonly extra_token_keys: readonly string[]; readonly device_flow: boolean; readonly device_code_url: string; readonly regex: string; From c5e0c9ca4f7d36b57b184003a7633a775e51678b Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Mon, 17 Jun 2024 17:15:45 -0400 Subject: [PATCH 4/4] Fix linting --- .../ExternalAuthSettingsPageView.stories.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/site/src/pages/DeploySettingsPage/ExternalAuthSettingsPage/ExternalAuthSettingsPageView.stories.tsx b/site/src/pages/DeploySettingsPage/ExternalAuthSettingsPage/ExternalAuthSettingsPageView.stories.tsx index 22a57140511f5..3ed1d8143738e 100644 --- a/site/src/pages/DeploySettingsPage/ExternalAuthSettingsPage/ExternalAuthSettingsPageView.stories.tsx +++ b/site/src/pages/DeploySettingsPage/ExternalAuthSettingsPage/ExternalAuthSettingsPageView.stories.tsx @@ -19,7 +19,6 @@ const meta: Meta = { app_installations_url: "", no_refresh: false, scopes: [], - extra_token_keys: [], device_flow: true, device_code_url: "", display_icon: "",