From ab6722e14ad20fcb5d733063c901d0fc136681a4 Mon Sep 17 00:00:00 2001 From: Ashish Kumar Singh <167757488+ashishsinghnr@users.noreply.github.com> Date: Wed, 19 Jun 2024 19:10:53 +0530 Subject: [PATCH 1/4] NR-281256 NEW_RELIC_LICENSE_KEY Priority and bypass Secrets Manager check if NEW_RELIC_LICENSE_KEY is set --- checks/sanity_check.go | 14 ++++++++++++-- credentials/credentials.go | 2 +- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/checks/sanity_check.go b/checks/sanity_check.go index 4b2f9ed..f9e7052 100644 --- a/checks/sanity_check.go +++ b/checks/sanity_check.go @@ -30,10 +30,17 @@ func sanityCheck(ctx context.Context, conf *config.Configuration, res *api.Regis envKeyExists := util.EnvVarExists("NEW_RELIC_LICENSE_KEY") var timeout = 1 * time.Second + + //Secret Manager ctxSecret, cancelSecret := context.WithTimeout(ctx, timeout) defer cancelSecret() - isSecretConfigured := credentials.IsSecretConfigured(ctxSecret, conf) + isSecretConfigured := false + if conf.LicenseKeySecretId != "" { + isSecretConfigured = credentials.IsSecretConfigured(ctxSecret, conf) + } + + // SSM Parameter ctxSSMParameter, cancelSSMParameter := context.WithTimeout(ctx, timeout) defer cancelSSMParameter() @@ -41,7 +48,6 @@ func sanityCheck(ctx context.Context, conf *config.Configuration, res *api.Regis if conf.LicenseKeySSMParameterName != "" { isSSMParameterConfigured = credentials.IsSSMParameterConfigured(ctxSSMParameter, conf) } - if isSecretConfigured && envKeyExists { return fmt.Errorf("There is both a AWS Secrets Manager secret and a NEW_RELIC_LICENSE_KEY environment variable set. Recommend removing the NEW_RELIC_LICENSE_KEY environment variable and using the AWS Secrets Manager secret.") @@ -55,5 +61,9 @@ func sanityCheck(ctx context.Context, conf *config.Configuration, res *api.Regis return fmt.Errorf("There is both a AWS Secrets Manager secret and a AWS Parameter Store parameter set. Recommend using just one.") } + if !envKeyExists || !isSecretConfigured || !isSSMParameterConfigured { + return fmt.Errorf("No configured license key found, attempting fallback to default AWS Secrets Manager secret with NEW_RELIC_LICENSE_KEY") + } + return nil } diff --git a/credentials/credentials.go b/credentials/credentials.go index 6911d25..f6013fc 100644 --- a/credentials/credentials.go +++ b/credentials/credentials.go @@ -120,7 +120,7 @@ func GetNewRelicLicenseKey(ctx context.Context, conf *config.Configuration) (str return envLicenseKey, nil } - util.Debugln("No configured license key found, attempting fallbacks") + util.Debugln("No configured license key found, attempting fallbacks to default") licenseKey, err := tryLicenseKeyFromSecret(ctx, defaultSecretId) if err == nil { From ebaddd9f13b9aa4c23fb9dd3e6d0be4dd6817469 Mon Sep 17 00:00:00 2001 From: Ashish Kumar Singh <167757488+ashishsinghnr@users.noreply.github.com> Date: Thu, 20 Jun 2024 00:08:59 +0530 Subject: [PATCH 2/4] NR-281256 [ NEW_RELIC_LICENSE_KEY Priority check ] Updated the test case --- checks/sanity_check.go | 4 +- checks/sanity_check_test.go | 90 ++++++++++++++++++------------------- 2 files changed, 47 insertions(+), 47 deletions(-) diff --git a/checks/sanity_check.go b/checks/sanity_check.go index f9e7052..87c15e3 100644 --- a/checks/sanity_check.go +++ b/checks/sanity_check.go @@ -61,8 +61,8 @@ func sanityCheck(ctx context.Context, conf *config.Configuration, res *api.Regis return fmt.Errorf("There is both a AWS Secrets Manager secret and a AWS Parameter Store parameter set. Recommend using just one.") } - if !envKeyExists || !isSecretConfigured || !isSSMParameterConfigured { - return fmt.Errorf("No configured license key found, attempting fallback to default AWS Secrets Manager secret with NEW_RELIC_LICENSE_KEY") + if !envKeyExists && !isSecretConfigured && !isSSMParameterConfigured { + return fmt.Errorf("No configured license key found, attempting fallback to default AWS Secrets Manager secret with NEW_RELIC_LICENSE_KEY.") } return nil diff --git a/checks/sanity_check_test.go b/checks/sanity_check_test.go index 5bb1a8c..9f8ba09 100644 --- a/checks/sanity_check_test.go +++ b/checks/sanity_check_test.go @@ -37,7 +37,7 @@ func (m mockSecretManager) GetSecretValueWithContext(_ context.Context, input *s type mockSSM struct { ssmiface.SSMAPI - validParameters []string + validParameters []string IsParameterCalled bool } @@ -68,12 +68,14 @@ func TestSanityCheck(t *testing.T) { ExpectedErr string }{ { - Name: "returns nil when nothing is configured", + Name: "returns error when nothing is configured", Conf: config.Configuration{}, Environment: map[string]string{}, SecretsManager: mockSecretManager{}, SSM: &mockSSM{}, + + ExpectedErr: "No configured license key found, attempting fallback to default AWS Secrets Manager secret with NEW_RELIC_LICENSE_KEY.", }, { Name: "returns nil when just the environment variable exists", @@ -197,48 +199,46 @@ func TestSanityCheck(t *testing.T) { } } - func TestSanityCheckSSMParameter(t *testing.T) { - ctx := context.Background() - - tests := []struct { - name string - ssmParameterName string - validParameters []string - expectParamCalled bool - expectedErr error - }{ - { - name: "SSM Parameter configured", - ssmParameterName: "parameter", - validParameters: []string{"parameter"}, - expectParamCalled: true, - expectedErr: nil, - }, - { - name: "SSM Parameter not configured", - expectParamCalled: false, - expectedErr: nil, - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - conf := config.Configuration{ - LicenseKeySSMParameterName: tc.ssmParameterName, - } - - mSSM := &mockSSM{ - validParameters: tc.validParameters, - } - - credentials.OverrideSSM(mSSM) - - err := sanityCheck(ctx, &conf, &api.RegistrationResponse{}, runtimeConfig{}) - - assert.Equal(t, tc.expectedErr, err, "Error from sanityCheck") - assert.Equal(t, tc.expectParamCalled, mSSM.IsParameterCalled, "Error in expected SSM parameter check") - }) - } -} + ctx := context.Background() + + tests := []struct { + name string + ssmParameterName string + validParameters []string + expectParamCalled bool + expectedErr error + }{ + { + name: "SSM Parameter configured", + ssmParameterName: "parameter", + validParameters: []string{"parameter"}, + expectParamCalled: true, + expectedErr: nil, + }, + { + name: "SSM Parameter not configured", + expectParamCalled: false, + expectedErr: nil, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + conf := config.Configuration{ + LicenseKeySSMParameterName: tc.ssmParameterName, + } + + mSSM := &mockSSM{ + validParameters: tc.validParameters, + } + credentials.OverrideSSM(mSSM) + + err := sanityCheck(ctx, &conf, &api.RegistrationResponse{}, runtimeConfig{}) + + assert.Equal(t, tc.expectedErr, err, "Error from sanityCheck") + assert.Equal(t, tc.expectParamCalled, mSSM.IsParameterCalled, "Error in expected SSM parameter check") + }) + } +} From 9a70d578a06bc28ed7384e365412dfae3811fa67 Mon Sep 17 00:00:00 2001 From: Ashish Kumar Singh <167757488+ashishsinghnr@users.noreply.github.com> Date: Thu, 20 Jun 2024 00:18:35 +0530 Subject: [PATCH 3/4] NR-281256 [ NEW_RELIC_LICENSE_KEY Priority check ] Corrected test case and removed duplicate --- checks/sanity_check_test.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/checks/sanity_check_test.go b/checks/sanity_check_test.go index 9f8ba09..350dc88 100644 --- a/checks/sanity_check_test.go +++ b/checks/sanity_check_test.go @@ -216,11 +216,6 @@ func TestSanityCheckSSMParameter(t *testing.T) { expectParamCalled: true, expectedErr: nil, }, - { - name: "SSM Parameter not configured", - expectParamCalled: false, - expectedErr: nil, - }, } for _, tc := range tests { From dbb6f1a678d41fb88add54d93b9ae31fd0abb864 Mon Sep 17 00:00:00 2001 From: Ashish Kumar Singh <167757488+ashishsinghnr@users.noreply.github.com> Date: Thu, 20 Jun 2024 16:32:25 +0530 Subject: [PATCH 4/4] NEW_RELIC_LICENSE_KEY Priority check NEW_RELIC_LICENSE_KEY Priority check --- checks/sanity_check.go | 2 +- checks/sanity_check_test.go | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/checks/sanity_check.go b/checks/sanity_check.go index 87c15e3..3ad67a8 100644 --- a/checks/sanity_check.go +++ b/checks/sanity_check.go @@ -62,7 +62,7 @@ func sanityCheck(ctx context.Context, conf *config.Configuration, res *api.Regis } if !envKeyExists && !isSecretConfigured && !isSSMParameterConfigured { - return fmt.Errorf("No configured license key found, attempting fallback to default AWS Secrets Manager secret with NEW_RELIC_LICENSE_KEY.") + util.Debugln("No configured license key found, attempting fallback to default AWS Secrets Manager secret with NEW_RELIC_LICENSE_KEY.") } return nil diff --git a/checks/sanity_check_test.go b/checks/sanity_check_test.go index 350dc88..9ac7828 100644 --- a/checks/sanity_check_test.go +++ b/checks/sanity_check_test.go @@ -68,14 +68,12 @@ func TestSanityCheck(t *testing.T) { ExpectedErr string }{ { - Name: "returns error when nothing is configured", + Name: "returns nil when nothing is configured", Conf: config.Configuration{}, Environment: map[string]string{}, SecretsManager: mockSecretManager{}, SSM: &mockSSM{}, - - ExpectedErr: "No configured license key found, attempting fallback to default AWS Secrets Manager secret with NEW_RELIC_LICENSE_KEY.", }, { Name: "returns nil when just the environment variable exists", @@ -216,6 +214,11 @@ func TestSanityCheckSSMParameter(t *testing.T) { expectParamCalled: true, expectedErr: nil, }, + { + name: "SSM Parameter not configured", + expectParamCalled: false, + expectedErr: nil, + }, } for _, tc := range tests {