Skip to content

Commit 58cd526

Browse files
fix(cli): revert addition of support for credentials check, #1783 (#1784)
1 parent 2becc2d commit 58cd526

File tree

4 files changed

+24
-152
lines changed

4 files changed

+24
-152
lines changed

internal/changeTracking/command_create_test.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,8 @@ package changeTracking
33
import (
44
"testing"
55

6-
"github.com/stretchr/testify/assert"
7-
86
"github.com/newrelic/newrelic-cli/internal/testcobra"
7+
"github.com/stretchr/testify/assert"
98
)
109

1110
func TestChangeTrackingCommand(t *testing.T) {

internal/install/command.go

Lines changed: 11 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import (
1515
configAPI "github.com/newrelic/newrelic-cli/internal/config/api"
1616
"github.com/newrelic/newrelic-cli/internal/install/types"
1717
"github.com/newrelic/newrelic-cli/internal/utils"
18-
"github.com/newrelic/newrelic-client-go/v2/pkg/apiaccess"
1918
nrErrors "github.com/newrelic/newrelic-client-go/v2/pkg/errors"
2019
nrRegion "github.com/newrelic/newrelic-client-go/v2/pkg/region"
2120
)
@@ -212,10 +211,10 @@ func checkNetwork() error {
212211
}
213212

214213
// Attempt to fetch and set a license key through 3 methods:
215-
// 1. NEW_RELIC_LICENSE_KEY environment variable (validates it belongs to the account),
216-
// 2. Active profile config.LicenseKey (validates it belongs to the account),
217-
// 3. API call (fetches a valid key for the account),
218-
// returns an error if all methods fail or if a provided key doesn't match the account.
214+
// 1. NEW_RELIC_LICENSE_KEY environment variable,
215+
// 2. Active profile config.LicenseKey,
216+
// 3. API call,
217+
// returns an error if all methods fail.
219218
func fetchLicenseKey() *types.DetailError {
220219
var licenseKey string
221220

@@ -229,43 +228,27 @@ func fetchLicenseKey() *types.DetailError {
229228
log.Debug("using license key: ", utils.Obfuscate(licenseKey))
230229
}
231230

232-
// Validate profile early since we'll need it for validation
233-
detailErr := validateProfile()
234-
if detailErr != nil {
235-
return detailErr
236-
}
237-
238-
accountID := configAPI.GetActiveProfileAccountID()
239-
240-
// Try environment variable first
241231
licenseKey = fetchLicenseKeyFromEnvironment()
242232

243233
if licenseKey != "" {
244-
log.Debug("validating that license key from environment belongs to account: ", accountID)
245-
// Validate that the environment license key belongs to the configured account
246-
if detailErr := validateLicenseKeyForAccount(licenseKey, accountID); detailErr != nil {
247-
return detailErr
248-
}
249-
log.Debug("license key from environment validated: belongs to account ", accountID)
250234
setLicenseKey(licenseKey)
251235
return nil
252236
}
253237

254-
// Try profile configuration
255238
licenseKey = fetchLicenseKeyFromProfile()
256239

257240
if licenseKey != "" {
258-
log.Debug("validating that license key from profile belongs to account: ", accountID)
259-
// Validate that the profile license key belongs to the configured account
260-
if detailErr := validateLicenseKeyForAccount(licenseKey, accountID); detailErr != nil {
261-
return detailErr
262-
}
263-
log.Debug("license key from profile validated: belongs to account ", accountID)
264241
setLicenseKey(licenseKey)
265242
return nil
266243
}
267244

268-
// Fetch licenseKey via API - this is already validated by definition since it comes from the account
245+
// fetch licenseKey via API
246+
detailErr := validateProfile()
247+
if detailErr != nil {
248+
log.Fatal(detailErr)
249+
}
250+
251+
accountID := configAPI.GetActiveProfileAccountID()
269252
maxTimeoutSeconds := config.DefaultMaxTimeoutSeconds
270253

271254
licenseKey, err := client.FetchLicenseKey(accountID, config.FlagProfileName, &maxTimeoutSeconds)
@@ -314,43 +297,3 @@ func fetchLicenseKeyFromProfile() string {
314297

315298
return ""
316299
}
317-
318-
// validateLicenseKeyForAccount verifies that a license key belongs to the specified account
319-
// to prevent installation validation failures where data is sent to one account but validated against another.
320-
func validateLicenseKeyForAccount(licenseKey string, accountID int) *types.DetailError {
321-
if client.NRClient == nil {
322-
return nil
323-
}
324-
325-
// Search for all ingest keys (including license keys) associated with the account
326-
params := apiaccess.APIAccessKeySearchQuery{
327-
Scope: apiaccess.APIAccessKeySearchScope{
328-
AccountIDs: []int{accountID},
329-
},
330-
Types: []apiaccess.APIAccessKeyType{
331-
apiaccess.APIAccessKeyTypeTypes.INGEST,
332-
},
333-
}
334-
335-
keys, err := client.NRClient.APIAccess.SearchAPIAccessKeysWithContext(utils.SignalCtx, params)
336-
337-
if err != nil {
338-
// If we can't verify, log a warning but don't fail the installation
339-
log.Warnf("unable to verify credential account match: %s", err)
340-
return nil
341-
}
342-
343-
// Check if the license key belongs to this account
344-
for _, k := range keys {
345-
if k.Key == licenseKey {
346-
return nil
347-
}
348-
}
349-
350-
// License key does not belong to this account
351-
log.Error("credential mismatch detected: license key does not belong to the configured account")
352-
return types.NewDetailError(
353-
types.EventTypes.CredentialAccountMismatch,
354-
fmt.Sprintf(types.ErrCredentialMismatch, utils.Obfuscate(licenseKey), accountID),
355-
)
356-
}

internal/install/command_test.go

Lines changed: 12 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import (
1313

1414
"github.com/newrelic/newrelic-cli/internal/install/types"
1515
"github.com/newrelic/newrelic-cli/internal/testcobra"
16-
"github.com/newrelic/newrelic-cli/internal/utils"
1716
)
1817

1918
func TestInstallCommand(t *testing.T) {
@@ -72,86 +71,22 @@ func TestValidateProfile(t *testing.T) {
7271
}
7372

7473
func TestFetchLicenseKey(t *testing.T) {
74+
okCase := func() *types.DetailError { return nil }()
7575
// TODO: Error case
76+
7677
// TODO: From API (mock?)
78+
7779
// TODO: From profile
7880

79-
// Save original environment to restore later
80-
origAccountID := os.Getenv("NEW_RELIC_ACCOUNT_ID")
81-
origAPIKey := os.Getenv("NEW_RELIC_API_KEY")
82-
origRegion := os.Getenv("NEW_RELIC_REGION")
83-
origLicenseKey := os.Getenv("NEW_RELIC_LICENSE_KEY")
84-
85-
// Restore environment after test
86-
defer func() {
87-
os.Setenv("NEW_RELIC_ACCOUNT_ID", origAccountID)
88-
os.Setenv("NEW_RELIC_API_KEY", origAPIKey)
89-
os.Setenv("NEW_RELIC_REGION", origRegion)
90-
os.Setenv("NEW_RELIC_LICENSE_KEY", origLicenseKey)
91-
}()
92-
93-
// ==================================================================================
94-
// TEST 1: License key matches account - NEW validation accepts it
95-
// ==================================================================================
96-
t.Run("LicenseKeyProvided_MatchingAccount", func(t *testing.T) {
97-
if origAccountID == "" || origAPIKey == "" || origRegion == "" || origLicenseKey == "" {
98-
t.Skip("Skipping: requires NEW_RELIC_ACCOUNT_ID, NEW_RELIC_API_KEY, NEW_RELIC_REGION, NEW_RELIC_LICENSE_KEY")
99-
}
100-
101-
os.Setenv("NEW_RELIC_ACCOUNT_ID", origAccountID)
102-
os.Setenv("NEW_RELIC_API_KEY", origAPIKey)
103-
os.Setenv("NEW_RELIC_REGION", origRegion)
104-
os.Setenv("NEW_RELIC_LICENSE_KEY", origLicenseKey)
105-
106-
err := fetchLicenseKey()
107-
108-
assert.Nil(t, err, "License key belonging to account should be accepted. Account: %s, Key: %s",
109-
origAccountID, utils.Obfuscate(origLicenseKey))
110-
assert.Equal(t, origLicenseKey, os.Getenv("NEW_RELIC_LICENSE_KEY"))
111-
})
112-
113-
// ==================================================================================
114-
// TEST 2: License key doesn't match account - NEW validation rejects it
115-
// ==================================================================================
116-
t.Run("LicenseKeyProvided_NonMatchingAccount", func(t *testing.T) {
117-
if origAccountID == "" || origAPIKey == "" || origRegion == "" || origLicenseKey == "" {
118-
t.Skip("Skipping: requires NEW_RELIC_ACCOUNT_ID, NEW_RELIC_API_KEY, NEW_RELIC_REGION, NEW_RELIC_LICENSE_KEY")
119-
}
120-
121-
wrongAccountID := "9999999"
122-
os.Setenv("NEW_RELIC_ACCOUNT_ID", wrongAccountID)
123-
os.Setenv("NEW_RELIC_API_KEY", origAPIKey)
124-
os.Setenv("NEW_RELIC_REGION", origRegion)
125-
os.Setenv("NEW_RELIC_LICENSE_KEY", origLicenseKey)
126-
127-
err := fetchLicenseKey()
128-
129-
assert.NotNil(t, err, "License key not belonging to account should be rejected")
130-
assert.Equal(t, types.EventTypes.CredentialAccountMismatch, err.EventName,
131-
"Expected CredentialAccountMismatch. Account: %s, Key: %s", wrongAccountID, utils.Obfuscate(origLicenseKey))
132-
assert.Contains(t, err.Error(), "credential mismatch detected")
133-
assert.Contains(t, err.Error(), wrongAccountID)
134-
})
135-
136-
// ==================================================================================
137-
// TEST 3: No license key provided - API fetch should succeed
138-
// ==================================================================================
139-
t.Run("NoLicenseKeyProvided_FetchesFromAPI", func(t *testing.T) {
140-
if origAccountID == "" || origAPIKey == "" || origRegion == "" {
141-
t.Skip("Skipping: requires NEW_RELIC_ACCOUNT_ID, NEW_RELIC_API_KEY, NEW_RELIC_REGION")
142-
}
143-
144-
os.Setenv("NEW_RELIC_ACCOUNT_ID", origAccountID)
145-
os.Setenv("NEW_RELIC_API_KEY", origAPIKey)
146-
os.Setenv("NEW_RELIC_REGION", origRegion)
147-
os.Unsetenv("NEW_RELIC_LICENSE_KEY")
148-
149-
err := fetchLicenseKey()
150-
151-
assert.Nil(t, err, "API fetch should succeed. Account: %s, API Key: %s",
152-
origAccountID, utils.Obfuscate(origAPIKey))
153-
assert.NotEmpty(t, os.Getenv("NEW_RELIC_LICENSE_KEY"), "License key should be fetched and set")
154-
})
81+
// From environment variable
82+
expect := "0123456789abcdefABCDEF0123456789abcdNRAL"
83+
os.Setenv("NEW_RELIC_LICENSE_KEY", expect)
84+
85+
err := fetchLicenseKey()
86+
assert.Equal(t, okCase, err)
87+
88+
actual := os.Getenv("NEW_RELIC_LICENSE_KEY")
89+
assert.Equal(t, expect, actual)
15590
}
15691

15792
func initSegmentMockServer() *httptest.Server {

internal/install/types/errors.go

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ var (
1717
ErrPostEvent = errors.New("there was a failure posting data to New Relic. Please try again later or contact New Relic support. For real-time platform status info visit https://status.newrelic.com/")
1818
ErrLicenseKey = errors.New("the configured license key is invalid for the configured account. Please set a valid license key with the `newrelic profile` command. For more details visit https://docs.newrelic.com/docs/apis/intro-apis/new-relic-api-keys/#ingest-license-key")
1919
ErrAgentControl = errors.New("agent control is installed, preventing the installation of this recipe")
20-
ErrCredentialMismatch = "credential mismatch detected: the license key %s does not belong to the configured account %d. This will cause installation validation to fail because data will be sent to one account but validated against another. Please ensure your API key, account ID, and license key are all from the same New Relic account. For more details about API keys visit https://docs.newrelic.com/docs/apis/intro-apis/new-relic-api-keys/"
2120
)
2221

2322
type EventType string
@@ -41,7 +40,6 @@ var EventTypes = struct {
4140
UnableToLocatePostedData EventType
4241
InvalidUserAPIKeyFormat EventType
4342
InvalidRegion EventType
44-
CredentialAccountMismatch EventType
4543
}{
4644
InstallStarted: "InstallStarted",
4745
AccountIDMissing: "AccountIDMissing",
@@ -61,7 +59,6 @@ var EventTypes = struct {
6159
OtherError: "OtherError",
6260
InvalidUserAPIKeyFormat: "InvalidUserAPIKeyFormat",
6361
InvalidRegion: "InvalidRegion",
64-
CredentialAccountMismatch: "CredentialAccountMismatch",
6562
}
6663

6764
func TryParseEventType(e string) (EventType, bool) {
@@ -96,8 +93,6 @@ func TryParseEventType(e string) (EventType, bool) {
9693
return EventTypes.InvalidUserAPIKeyFormat, true
9794
case "InvalidRegion":
9895
return EventTypes.InvalidRegion, true
99-
case "CredentialAccountMismatch":
100-
return EventTypes.CredentialAccountMismatch, true
10196
}
10297

10398
return "", false

0 commit comments

Comments
 (0)