From 727911a830d7c81b10e87a9e7b9390fdbd3ebe69 Mon Sep 17 00:00:00 2001 From: Eyal Delarea Date: Wed, 13 Nov 2024 14:59:16 +0200 Subject: [PATCH] Add commands to usage report (#1299) --- artifactory/commands/buildinfo/publish.go | 7 +++ common/commands/command.go | 4 +- common/commands/config.go | 27 ++++++++++- common/commands/config_test.go | 59 +++++++++++++++++++++++ utils/coreutils/coreconsts.go | 7 +++ 5 files changed, 102 insertions(+), 2 deletions(-) diff --git a/artifactory/commands/buildinfo/publish.go b/artifactory/commands/buildinfo/publish.go index a33d93508..ab016595f 100644 --- a/artifactory/commands/buildinfo/publish.go +++ b/artifactory/commands/buildinfo/publish.go @@ -70,6 +70,13 @@ func (bpc *BuildPublishCommand) IsDetailedSummary() bool { } func (bpc *BuildPublishCommand) CommandName() string { + autoPublishedTriggered, err := clientutils.GetBoolEnvValue(coreutils.UsageAutoPublishedBuild, false) + if err != nil { + log.Warn("Failed to get the value of the environment variable: " + coreutils.UsageAutoPublishedBuild + ". " + err.Error()) + } + if autoPublishedTriggered { + return "rt_build_publish_auto" + } return "rt_build_publish" } diff --git a/common/commands/command.go b/common/commands/command.go index 074876d42..34d4fc6bf 100644 --- a/common/commands/command.go +++ b/common/commands/command.go @@ -58,5 +58,7 @@ func reportUsage(command Command, channel chan<- bool) { // Set to true when the report usage func exits func signalReportUsageFinished(ch chan<- bool) { - ch <- true + if ch != nil { + ch <- true + } } diff --git a/common/commands/config.go b/common/commands/config.go index d542593d0..0cec91e0d 100644 --- a/common/commands/config.go +++ b/common/commands/config.go @@ -41,6 +41,13 @@ const ( WebLogin AuthenticationMethod = "Web Login" ) +const ( + // Indicates that the config command uses OIDC authentication. + configOidcCommandName = "config_oidc" + // Default config command name. + configCommandName = "config" +) + // Internal golang locking for the same process. var mutex sync.Mutex @@ -143,7 +150,25 @@ func (cc *ConfigCommand) ServerDetails() (*config.ServerDetails, error) { } func (cc *ConfigCommand) CommandName() string { - return "config" + oidcConfigured, err := clientUtils.GetBoolEnvValue(coreutils.UsageOidcConfigured, false) + if err != nil { + log.Warn("Failed to get the value of the environment variable: " + coreutils.UsageAutoPublishedBuild + ". " + err.Error()) + } + if oidcConfigured { + return configOidcCommandName + } + return configCommandName +} + +// ExecAndReportUsage runs the ConfigCommand and then triggers a usage report if needed, +// Report usage only if OIDC integration was used +// Usage must be sent after command execution as we need the server details to be set. +func (cc *ConfigCommand) ExecAndReportUsage() (err error) { + if err = cc.Run(); err != nil { + return + } + reportUsage(cc, nil) + return } func (cc *ConfigCommand) config() error { diff --git a/common/commands/config_test.go b/common/commands/config_test.go index 029d6250a..131f8cff1 100644 --- a/common/commands/config_test.go +++ b/common/commands/config_test.go @@ -2,6 +2,7 @@ package commands import ( "encoding/json" + testsUtils "github.com/jfrog/jfrog-client-go/utils/tests" "os" "testing" @@ -344,6 +345,64 @@ func TestImport(t *testing.T) { assert.Equal(t, "password", serverDetails.GetPassword()) } +func TestCommandName(t *testing.T) { + // Clean up + defer func() { + testsUtils.UnSetEnvAndAssert(t, coreutils.UsageOidcConfigured) + }() + cc := NewConfigCommand(AddOrEdit, testServerId) + // Test when the environment variable is not set + assert.Equal(t, configCommandName, cc.CommandName()) + // Test when the environment variable is set + testsUtils.SetEnvWithCallbackAndAssert(t, coreutils.UsageOidcConfigured, "true") + assert.Equal(t, configOidcCommandName, cc.CommandName()) +} + +func TestConfigCommand_ExecAndReportUsage(t *testing.T) { + defer func() { + testsUtils.UnSetEnvAndAssert(t, coreutils.UsageOidcConfigured) + }() + // Define test scenarios + testCases := []struct { + name string + envVarValue string // Environment variable value to set (or empty to unset) + expectedName string // Expected command name + expectError bool // Whether an error is expected + }{ + { + name: "With usage report", + envVarValue: "TRUE", + expectedName: configOidcCommandName, + }, + { + name: "Without usage report", + envVarValue: "", // Empty to unset the environment variable + expectedName: configCommandName, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // Set or unset the environment variable + if tc.envVarValue != "" { + err := os.Setenv(coreutils.UsageOidcConfigured, tc.envVarValue) + assert.NoError(t, err) + } else { + err := os.Unsetenv(coreutils.UsageOidcConfigured) + assert.NoError(t, err) + } + + // Initialize the command and check the expected command name + cc := NewConfigCommand(AddOrEdit, testServerId) + assert.Equal(t, tc.expectedName, cc.CommandName()) + + // Execute and validate no errors + err := cc.ExecAndReportUsage() + assert.NoError(t, err) + }) + } +} + func testExportImport(t *testing.T, inputDetails *config.ServerDetails) { configToken, err := config.Export(inputDetails) assert.NoError(t, err) diff --git a/utils/coreutils/coreconsts.go b/utils/coreutils/coreconsts.go index 2e1eba07c..9e83bc8b0 100644 --- a/utils/coreutils/coreconsts.go +++ b/utils/coreutils/coreconsts.go @@ -50,6 +50,13 @@ const ( ServerID = "JFROG_CLI_SERVER_ID" TransitiveDownload = "JFROG_CLI_TRANSITIVE_DOWNLOAD" + // These environment variables are used to adjust command names for more detailed tracking in the usage report. + // Set by the setup-jfrog-cli GitHub Action to identify specific command usage scenarios. + // True if an automatic build publication was triggered. + UsageAutoPublishedBuild = "JFROG_CLI_USAGE_AUTO_BUILD_PUBLISHED" + // True if the JFrog platform was configured using OIDC integration. + UsageOidcConfigured = "JFROG_CLI_USAGE_CONFIG_OIDC" + // Deprecated and replaced with TransitiveDownload TransitiveDownloadExperimental = "JFROG_CLI_TRANSITIVE_DOWNLOAD_EXPERIMENTAL" )