diff --git a/pkg/base/testing_knobs.go b/pkg/base/testing_knobs.go index 7148fc54e43d..0cc15c3a7752 100644 --- a/pkg/base/testing_knobs.go +++ b/pkg/base/testing_knobs.go @@ -56,4 +56,5 @@ type TestingKnobs struct { TableStatsKnobs ModuleTestingKnobs Insights ModuleTestingKnobs TableMetadata ModuleTestingKnobs + LicenseTestingKnobs ModuleTestingKnobs } diff --git a/pkg/ccl/utilccl/license_check.go b/pkg/ccl/utilccl/license_check.go index a1de67cd2ae4..42e9fbc905e8 100644 --- a/pkg/ccl/utilccl/license_check.go +++ b/pkg/ccl/utilccl/license_check.go @@ -187,7 +187,7 @@ var GetLicenseTTL = func( st *cluster.Settings, ts timeutil.TimeSource, ) int64 { - license, err := getLicense(st) + license, err := GetLicense(st) if err != nil { log.Errorf(ctx, "unable to find license: %v", err) return 0 @@ -209,7 +209,7 @@ func checkEnterpriseEnabledAt( if atomic.LoadInt32(&enterpriseStatus) == enterpriseEnabled { return nil } - license, err := getLicense(st) + license, err := GetLicense(st) if err != nil { return err } @@ -240,10 +240,10 @@ func checkEnterpriseEnabledAt( return nil } -// getLicense fetches the license from the given settings, using Settings.Cache +// GetLicense fetches the license from the given settings, using Settings.Cache // to cache the decoded license (if any). The returned license must not be // modified by the caller. -func getLicense(st *cluster.Settings) (*licenseccl.License, error) { +func GetLicense(st *cluster.Settings) (*licenseccl.License, error) { str := enterpriseLicense.Get(&st.SV) if str == "" { return nil, nil @@ -263,7 +263,7 @@ func getLicense(st *cluster.Settings) (*licenseccl.License, error) { // GetLicenseType returns the license type. func GetLicenseType(st *cluster.Settings) (string, error) { - license, err := getLicense(st) + license, err := GetLicense(st) if err != nil { return "", err } else if license == nil { @@ -274,7 +274,7 @@ func GetLicenseType(st *cluster.Settings) (string, error) { // GetLicenseEnvironment returns the license environment. func GetLicenseEnvironment(st *cluster.Settings) (string, error) { - license, err := getLicense(st) + license, err := GetLicense(st) if err != nil { return "", err } else if license == nil { @@ -354,7 +354,7 @@ func RegisterCallbackOnLicenseChange( // The isChange parameter indicates whether the license is actually being updated, // as opposed to merely refreshing the current license. refreshFunc := func(ctx context.Context, isChange bool) { - lic, err := getLicense(st) + lic, err := GetLicense(st) if err != nil { log.Errorf(ctx, "unable to refresh license enforcer for license change: %v", err) return diff --git a/pkg/ccl/utilccl/license_check_test.go b/pkg/ccl/utilccl/license_check_test.go index 1b7f0b64fc2a..9ee600123604 100644 --- a/pkg/ccl/utilccl/license_check_test.go +++ b/pkg/ccl/utilccl/license_check_test.go @@ -14,7 +14,6 @@ import ( "github.com/cockroachdb/cockroach/pkg/base" "github.com/cockroachdb/cockroach/pkg/ccl/utilccl/licenseccl" - "github.com/cockroachdb/cockroach/pkg/server" "github.com/cockroachdb/cockroach/pkg/server/license" "github.com/cockroachdb/cockroach/pkg/settings" "github.com/cockroachdb/cockroach/pkg/settings/cluster" @@ -386,12 +385,10 @@ func TestRefreshLicenseEnforcerOnLicenseChange(t *testing.T) { // We are changing a cluster setting that can only be done at the system tenant. DefaultTestTenant: base.TestIsSpecificToStorageLayerAndNeedsASystemTenant, Knobs: base.TestingKnobs{ - Server: &server.TestingKnobs{ - LicenseTestingKnobs: license.TestingKnobs{ - Enable: true, - SkipDisable: true, - OverrideStartTime: &ts1, - }, + LicenseTestingKnobs: &license.TestingKnobs{ + Enable: true, + SkipDisable: true, + OverrideStartTime: &ts1, }, }, }) diff --git a/pkg/server/application_api/telemetry_test.go b/pkg/server/application_api/telemetry_test.go index b4e100341af2..31d7681ac212 100644 --- a/pkg/server/application_api/telemetry_test.go +++ b/pkg/server/application_api/telemetry_test.go @@ -14,7 +14,6 @@ import ( "github.com/cockroachdb/cockroach/pkg/base" "github.com/cockroachdb/cockroach/pkg/ccl/utilccl/licenseccl" - "github.com/cockroachdb/cockroach/pkg/server" "github.com/cockroachdb/cockroach/pkg/server/apiconstants" "github.com/cockroachdb/cockroach/pkg/server/diagnostics" "github.com/cockroachdb/cockroach/pkg/server/diagnostics/diagnosticspb" @@ -137,11 +136,9 @@ func TestThrottlingMetadata(t *testing.T) { s := serverutils.StartCluster(t, 3, base.TestClusterArgs{ ServerArgs: base.TestServerArgs{ Knobs: base.TestingKnobs{ - Server: &server.TestingKnobs{ - LicenseTestingKnobs: license.TestingKnobs{ - Enable: true, - OverrideStartTime: &testtime, - }, + LicenseTestingKnobs: &license.TestingKnobs{ + Enable: true, + OverrideStartTime: &testtime, }, }, }, diff --git a/pkg/server/diagnostics/BUILD.bazel b/pkg/server/diagnostics/BUILD.bazel index 0de3099dbdf2..e4b9acb83f31 100644 --- a/pkg/server/diagnostics/BUILD.bazel +++ b/pkg/server/diagnostics/BUILD.bazel @@ -13,6 +13,8 @@ go_library( deps = [ "//pkg/base", "//pkg/build", + "//pkg/ccl/utilccl", + "//pkg/ccl/utilccl/licenseccl", "//pkg/config/zonepb", "//pkg/keys", "//pkg/kv", @@ -61,14 +63,17 @@ go_test( deps = [ "//pkg/base", "//pkg/build", + "//pkg/ccl/utilccl/licenseccl", "//pkg/security/securityassets", "//pkg/security/securitytest", "//pkg/server", + "//pkg/server/diagnostics/diagnosticspb", "//pkg/testutils/diagutils", "//pkg/testutils/serverutils", "//pkg/util/cloudinfo", "//pkg/util/leaktest", "//pkg/util/log", + "//pkg/util/uuid", "@com_github_mitchellh_reflectwalk//:reflectwalk", "@com_github_stretchr_testify//require", ], diff --git a/pkg/server/diagnostics/diagnostics.go b/pkg/server/diagnostics/diagnostics.go index 6f3657ee5ad9..eae7ee21ff72 100644 --- a/pkg/server/diagnostics/diagnostics.go +++ b/pkg/server/diagnostics/diagnostics.go @@ -12,10 +12,12 @@ import ( "strconv" "time" + "github.com/cockroachdb/cockroach/pkg/ccl/utilccl/licenseccl" "github.com/cockroachdb/cockroach/pkg/roachpb" "github.com/cockroachdb/cockroach/pkg/server/diagnostics/diagnosticspb" "github.com/cockroachdb/cockroach/pkg/util/cloudinfo" "github.com/cockroachdb/cockroach/pkg/util/envutil" + "github.com/cockroachdb/cockroach/pkg/util/log" "github.com/cockroachdb/cockroach/pkg/util/syncutil" "github.com/cockroachdb/cockroach/pkg/util/system" "github.com/cockroachdb/cockroach/pkg/util/uuid" @@ -71,6 +73,7 @@ type ClusterInfo struct { TenantID roachpb.TenantID IsInsecure bool IsInternal bool + License *licenseccl.License } // addInfoToURL sets query parameters on the URL used to report diagnostics. If @@ -94,10 +97,33 @@ func addInfoToURL( q.Set("nodeid", strconv.Itoa(int(nodeID))) } + environment := "" + licenseExpiry := "" + organizationID := "" + licenseID := "" + if clusterInfo.License != nil { + license := clusterInfo.License + environment = license.Environment.String() + if license.OrganizationId != nil { + organizationUUID, err := uuid.FromBytes(license.OrganizationId) + if err != nil { + log.Infof(context.Background(), "unexpected error parsing organization id from license %s", err) + } + organizationID = organizationUUID.String() + } + if license.LicenseId != nil { + licenseExpiry = strconv.Itoa(int(license.ValidUntilUnixSec)) + licenseUUID, err := uuid.FromBytes(license.LicenseId) + if err != nil { + log.Infof(context.Background(), "unexpected error parsing organization id from license %s", err) + } + licenseID = licenseUUID.String() + } + } + b := env.Build q.Set("sqlid", strconv.Itoa(int(sqlInfo.SQLInstanceID))) q.Set("uptime", strconv.Itoa(int(sqlInfo.Uptime))) - q.Set("licensetype", env.LicenseType) q.Set("version", b.Tag) q.Set("platform", b.Platform) q.Set("uuid", clusterInfo.StorageClusterID.String()) @@ -107,6 +133,12 @@ func addInfoToURL( q.Set("internal", strconv.FormatBool(clusterInfo.IsInternal)) q.Set("buildchannel", b.Channel) q.Set("envchannel", b.EnvChannel) + // license type must come from the environment because it uses the base package. + q.Set("licensetype", env.LicenseType) + q.Set("organization_id", organizationID) + q.Set("license_id", licenseID) + q.Set("license_expiry_seconds", licenseExpiry) + q.Set("environment", environment) result.RawQuery = q.Encode() return &result } diff --git a/pkg/server/diagnostics/reporter.go b/pkg/server/diagnostics/reporter.go index 44b7d41108cb..724ddeef56db 100644 --- a/pkg/server/diagnostics/reporter.go +++ b/pkg/server/diagnostics/reporter.go @@ -17,6 +17,8 @@ import ( "github.com/cockroachdb/cockroach/pkg/base" "github.com/cockroachdb/cockroach/pkg/build" + "github.com/cockroachdb/cockroach/pkg/ccl/utilccl" + "github.com/cockroachdb/cockroach/pkg/ccl/utilccl/licenseccl" "github.com/cockroachdb/cockroach/pkg/config/zonepb" "github.com/cockroachdb/cockroach/pkg/keys" "github.com/cockroachdb/cockroach/pkg/kv" @@ -142,7 +144,13 @@ func (r *Reporter) ReportDiagnostics(ctx context.Context) { report := r.CreateReport(ctx, telemetry.ResetCounts) - url := r.buildReportingURL(report) + license, err := utilccl.GetLicense(r.Settings) + if err != nil { + if log.V(2) { + log.Warningf(ctx, "failed to retrieve license while reporting diagnostics: %v", err) + } + } + url := r.buildReportingURL(report, license) if url == nil { return } @@ -377,13 +385,20 @@ func (r *Reporter) collectSchemaInfo(ctx context.Context) ([]descpb.TableDescrip // buildReportingURL creates a URL to report diagnostics. // If an empty updates URL is set (via empty environment variable), returns nil. -func (r *Reporter) buildReportingURL(report *diagnosticspb.DiagnosticReport) *url.URL { +func (r *Reporter) buildReportingURL( + report *diagnosticspb.DiagnosticReport, license *licenseccl.License, +) *url.URL { + if license == nil { + license = &licenseccl.License{} + } + clusterInfo := ClusterInfo{ StorageClusterID: r.StorageClusterID(), LogicalClusterID: r.LogicalClusterID(), TenantID: r.TenantID, IsInsecure: r.Config.Insecure, IsInternal: sql.ClusterIsInternal(&r.Settings.SV), + License: license, } url := reportingURL diff --git a/pkg/server/diagnostics/reporter_test.go b/pkg/server/diagnostics/reporter_test.go index 93e8d3c41abd..d67aa625938b 100644 --- a/pkg/server/diagnostics/reporter_test.go +++ b/pkg/server/diagnostics/reporter_test.go @@ -6,10 +6,18 @@ package diagnostics import ( + "context" + "fmt" "testing" + "github.com/cockroachdb/cockroach/pkg/base" + build "github.com/cockroachdb/cockroach/pkg/build" + "github.com/cockroachdb/cockroach/pkg/ccl/utilccl/licenseccl" + "github.com/cockroachdb/cockroach/pkg/server/diagnostics/diagnosticspb" + "github.com/cockroachdb/cockroach/pkg/testutils/serverutils" "github.com/cockroachdb/cockroach/pkg/util/leaktest" "github.com/cockroachdb/cockroach/pkg/util/log" + "github.com/cockroachdb/cockroach/pkg/util/uuid" "github.com/mitchellh/reflectwalk" "github.com/stretchr/testify/require" ) @@ -42,3 +50,76 @@ func TestStringRedactor_Primitive(t *testing.T) { require.Equal(t, "_", *foo.B) require.Equal(t, "string 3", foo.C["3"]) } + +func TestBuildReportingURL(t *testing.T) { + defer leaktest.AfterTest(t)() + defer log.Scope(t).Close(t) + srv := serverutils.StartServerOnly(t, base.TestServerArgs{}) + defer srv.Stopper().Stop(context.Background()) + + report := &diagnosticspb.DiagnosticReport{ + Env: diagnosticspb.Environment{ + LicenseType: "Enterprise", + Build: build.Info{ + Tag: "tag", + Platform: "platform", + Channel: "buildchannel", + EnvChannel: "envchannel", + }, + }, + Node: diagnosticspb.NodeInfo{ + NodeID: 1, + }, + SQL: diagnosticspb.SQLInstanceInfo{ + SQLInstanceID: 2, + Uptime: 3, + }, + } + licenseID, err := uuid.FromString("abc362b1-4f67-4bc0-b7dd-5628e49d2cba") + require.NoError(t, err) + organizationID, err := uuid.FromString("123362b1-4f67-4bc0-b7dd-5628e49d2321") + require.NoError(t, err) + license := &licenseccl.License{ + ValidUntilUnixSec: 4, + Type: licenseccl.License_Enterprise, + Environment: 1, + LicenseId: licenseID.GetBytes(), + OrganizationId: organizationID.GetBytes(), + } + r := srv.DiagnosticsReporter().(*Reporter) + url := r.buildReportingURL(report, license) + logicalClusterUUID := r.LogicalClusterID() + storageClusterUUID := r.StorageClusterID() + require.Equal(t, fmt.Sprintf(`https://register.cockroachdb.com/api/clusters/report?buildchannel=buildchannel&envchannel=envchannel&environment=production&insecure=false&internal=false&license_expiry_seconds=4&license_id=abc362b1-4f67-4bc0-b7dd-5628e49d2cba&licensetype=Enterprise&logical_uuid=%s&nodeid=1&organization_id=123362b1-4f67-4bc0-b7dd-5628e49d2321&platform=platform&sqlid=2&tenantid=%s&uptime=3&uuid=%s&version=tag`, logicalClusterUUID, r.TenantID.String(), storageClusterUUID), url.String()) +} + +func TestBuildReportingURLNoLicense(t *testing.T) { + defer leaktest.AfterTest(t)() + defer log.Scope(t).Close(t) + srv := serverutils.StartServerOnly(t, base.TestServerArgs{}) + defer srv.Stopper().Stop(context.Background()) + + report := &diagnosticspb.DiagnosticReport{ + Env: diagnosticspb.Environment{ + LicenseType: "OSS", + Build: build.Info{ + Tag: "tag", + Platform: "platform", + Channel: "buildchannel", + EnvChannel: "envchannel", + }, + }, + Node: diagnosticspb.NodeInfo{ + NodeID: 1, + }, + SQL: diagnosticspb.SQLInstanceInfo{ + SQLInstanceID: 2, + Uptime: 3, + }, + } + r := srv.DiagnosticsReporter().(*Reporter) + url := r.buildReportingURL(report, nil) + logicalClusterUUID := r.LogicalClusterID() + storageClusterUUID := r.StorageClusterID() + require.Equal(t, fmt.Sprintf(`https://register.cockroachdb.com/api/clusters/report?buildchannel=buildchannel&envchannel=envchannel&environment=&insecure=false&internal=false&license_expiry_seconds=&license_id=&licensetype=OSS&logical_uuid=%s&nodeid=1&organization_id=&platform=platform&sqlid=2&tenantid=%s&uptime=3&uuid=%s&version=tag`, logicalClusterUUID, r.TenantID.String(), storageClusterUUID), url.String()) +} diff --git a/pkg/server/license/enforcer.go b/pkg/server/license/enforcer.go index db2d5268c5fe..3584d9b359f4 100644 --- a/pkg/server/license/enforcer.go +++ b/pkg/server/license/enforcer.go @@ -124,6 +124,9 @@ type TestingKnobs struct { OverwriteClusterInitGracePeriodTS bool } +// ModuleTestingKnobs is part of the base.ModuleTestingKnobs interface. +func (*TestingKnobs) ModuleTestingKnobs() {} + // TelemetryStatusReporter is the interface we use to find the last ping // time for telemetry reporting. type TelemetryStatusReporter interface { diff --git a/pkg/server/license/enforcer_test.go b/pkg/server/license/enforcer_test.go index 376536ed0176..c83a50ecbb52 100644 --- a/pkg/server/license/enforcer_test.go +++ b/pkg/server/license/enforcer_test.go @@ -13,7 +13,6 @@ import ( "time" "github.com/cockroachdb/cockroach/pkg/base" - "github.com/cockroachdb/cockroach/pkg/server" "github.com/cockroachdb/cockroach/pkg/server/license" "github.com/cockroachdb/cockroach/pkg/sql" "github.com/cockroachdb/cockroach/pkg/sql/catalog/descs" @@ -48,11 +47,9 @@ func TestClusterInitGracePeriod_NoOverwrite(t *testing.T) { ctx := context.Background() srv := serverutils.StartServerOnly(t, base.TestServerArgs{ Knobs: base.TestingKnobs{ - Server: &server.TestingKnobs{ - LicenseTestingKnobs: license.TestingKnobs{ - Enable: true, - OverrideStartTime: &ts1, - }, + LicenseTestingKnobs: &license.TestingKnobs{ + Enable: true, + OverrideStartTime: &ts1, }, }, }) @@ -101,11 +98,9 @@ func TestClusterInitGracePeriod_NewClusterEstimation(t *testing.T) { ctx := context.Background() srv := serverutils.StartServerOnly(t, base.TestServerArgs{ Knobs: base.TestingKnobs{ - Server: &server.TestingKnobs{ - LicenseTestingKnobs: license.TestingKnobs{ - Enable: true, - OverrideStartTime: &ts1, - }, + LicenseTestingKnobs: &license.TestingKnobs{ + Enable: true, + OverrideStartTime: &ts1, }, }, }) @@ -290,20 +285,18 @@ func TestThrottleErrorMsg(t *testing.T) { srv, sqlDB, _ := serverutils.StartServer(t, base.TestServerArgs{ Knobs: base.TestingKnobs{ - Server: &server.TestingKnobs{ - LicenseTestingKnobs: license.TestingKnobs{ - Enable: true, - // The mock server we bring up is single-node, which disables all - // throttling checks. We need to avoid that for this test to verify - // the throttle message. - SkipDisable: true, - // We are going to modify the throttle check timestamp in each test - // unit. - OverrideThrottleCheckTime: throttleCheckTS, - // And we will modify what is the max open transactions to force us - // over the limit. - OverrideMaxOpenTransactions: &maxOpenTransactions, - }, + LicenseTestingKnobs: &license.TestingKnobs{ + Enable: true, + // The mock server we bring up is single-node, which disables all + // throttling checks. We need to avoid that for this test to verify + // the throttle message. + SkipDisable: true, + // We are going to modify the throttle check timestamp in each test + // unit. + OverrideThrottleCheckTime: throttleCheckTS, + // And we will modify what is the max open transactions to force us + // over the limit. + OverrideMaxOpenTransactions: &maxOpenTransactions, }, }, }) diff --git a/pkg/server/server_sql.go b/pkg/server/server_sql.go index 16c4d934f237..f09088c1507a 100644 --- a/pkg/server/server_sql.go +++ b/pkg/server/server_sql.go @@ -1924,8 +1924,8 @@ func (s *SQLServer) startLicenseEnforcer(ctx context.Context, knobs base.Testing license.WithSystemTenant(s.execCfg.Codec.ForSystemTenant()), license.WithTelemetryStatusReporter(s.diagnosticsReporter), } - if knobs.Server != nil { - opts = append(opts, license.WithTestingKnobs(&knobs.Server.(*TestingKnobs).LicenseTestingKnobs)) + if knobs.LicenseTestingKnobs != nil { + opts = append(opts, license.WithTestingKnobs(knobs.LicenseTestingKnobs.(*license.TestingKnobs))) } err := startup.RunIdempotentWithRetry(ctx, s.stopper.ShouldQuiesce(), "license enforcer start", func(ctx context.Context) error { diff --git a/pkg/server/testing_knobs.go b/pkg/server/testing_knobs.go index 6e0fe9533515..57b3d3ef2cc5 100644 --- a/pkg/server/testing_knobs.go +++ b/pkg/server/testing_knobs.go @@ -15,7 +15,6 @@ import ( "github.com/cockroachdb/cockroach/pkg/roachpb" "github.com/cockroachdb/cockroach/pkg/rpc" "github.com/cockroachdb/cockroach/pkg/server/diagnostics" - "github.com/cockroachdb/cockroach/pkg/server/license" "github.com/cockroachdb/cockroach/pkg/storage/fs" "github.com/cockroachdb/cockroach/pkg/util/hlc" ) @@ -42,8 +41,6 @@ type TestingKnobs struct { ContextTestingKnobs rpc.ContextTestingKnobs // DiagnosticsTestingKnobs allows customization of diagnostics testing knobs. DiagnosticsTestingKnobs diagnostics.TestingKnobs - // LicenseTestingKnobs allows customization of license testing knobs. - LicenseTestingKnobs license.TestingKnobs // If set, use this listener for RPC (and possibly SQL, depending on // the SplitListenSQL setting), instead of binding a new listener. diff --git a/pkg/server/testserver.go b/pkg/server/testserver.go index a210158c75d3..76f0440e1d7f 100644 --- a/pkg/server/testserver.go +++ b/pkg/server/testserver.go @@ -620,7 +620,7 @@ func (ts *testServer) startDefaultTestTenant( if ts.params.Knobs.Server != nil { params.TestingKnobs.Server.(*TestingKnobs).DiagnosticsTestingKnobs = ts.params.Knobs.Server.(*TestingKnobs).DiagnosticsTestingKnobs - params.TestingKnobs.Server.(*TestingKnobs).LicenseTestingKnobs = ts.params.Knobs.Server.(*TestingKnobs).LicenseTestingKnobs + params.TestingKnobs.LicenseTestingKnobs = ts.params.Knobs.LicenseTestingKnobs } return ts.StartTenant(ctx, params) } @@ -637,7 +637,7 @@ func (ts *testServer) getSharedProcessDefaultTenantArgs() base.TestSharedProcess args.Knobs.Server = &TestingKnobs{} if ts.params.Knobs.Server != nil { args.Knobs.Server.(*TestingKnobs).DiagnosticsTestingKnobs = ts.params.Knobs.Server.(*TestingKnobs).DiagnosticsTestingKnobs - args.Knobs.Server.(*TestingKnobs).LicenseTestingKnobs = ts.params.Knobs.Server.(*TestingKnobs).LicenseTestingKnobs + args.Knobs.LicenseTestingKnobs = ts.params.Knobs.LicenseTestingKnobs } return args } diff --git a/pkg/upgrade/upgrades/v24_3_check_license_violation_test.go b/pkg/upgrade/upgrades/v24_3_check_license_violation_test.go index 363e7b8079e7..2fcbe2aa41a8 100644 --- a/pkg/upgrade/upgrades/v24_3_check_license_violation_test.go +++ b/pkg/upgrade/upgrades/v24_3_check_license_violation_test.go @@ -48,10 +48,10 @@ func TestCheckLicenseViolations(t *testing.T) { Server: &server.TestingKnobs{ DisableAutomaticVersionUpgrade: make(chan struct{}), ClusterVersionOverride: v1, - LicenseTestingKnobs: license.TestingKnobs{ - Enable: true, - SkipDisable: true, - }, + }, + LicenseTestingKnobs: &license.TestingKnobs{ + Enable: true, + SkipDisable: true, }, // Make the upgrade faster by accelerating jobs. JobsTestingKnobs: jobs.NewTestingKnobsWithShortIntervals(),