Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add service metrics for Secrets requested and stored #376

Merged

Conversation

lenny-goodell
Copy link
Member

@lenny-goodell lenny-goodell commented Oct 5, 2022

applies to #374

Signed-off-by: Leonard Goodell [email protected]

If your build fails due to your commit message not passing the build checks, please review the guidelines here: https://github.com/edgexfoundry/go-mod-bootstrap/blob/main/.github/Contributing.md

PR Checklist

Please check if your PR fulfills the following requirements:

  • I am not introducing a breaking change (if you are, flag in conventional commit message with BREAKING CHANGE: describing the break)
  • I am not introducing a new dependency (add notes below if you are)
  • I have added unit tests for the new feature or bug fix (if not, why?) N/A
  • I have fully tested (add details below) this the new feature or bug fix (if not, why?)
  • I have opened a PR for the related docs change (if not, why?)
    TBD

Testing Instructions

Non-secure mode

Run non-secure EdgeX stack
Run ACS with metrics-influxdb profile from command line with DEBUG logging

WRITABLE_LOGLEVEL=DEBUG EDGEX_SECURITY_SECRET_STORE=false ./app-service-configurable -p=metrics-influxdb | grep Transformed Metric

Build and run app template in App SDk with go-mod-bootstrap from this branch
Disable all exiting metrics in the app template config file
Add the following metrics enabled

    [Writable.Telemetry.Metrics] # All service's metric names must be present in this list.
    SecuritySecretsRequested = true
    SecuritySecretsStored = true

Run app template service from command-line with -cp option so it uses Consul for config and DEBUG logging

WRITABLE_LOGLEVEL=DEBUG  DGEX_SECURITY_SECRET_STORE=false ./new-app-service -cp

Verify ever 30 secs it logs the following:

msg="Publish 2 metrics to the 'edgex/telemetry/app-new-service' base topic"
msg="Reported metrics..."

Verify ASC logs have the following:

msg="Transformed Metric to 'SecuritySecretsRequested,service=app-new-service counter-count=1 1664992310177494700\n'
msg="Transformed Metric to SecuritySecretsStored,service=app-new-service counter-count=0 1664992310177494700\n'

From Consul change the value at edgex/appservices/2.0/app-new-service/Writable/InsecureSecrets/HTTPS/Secrets/key and save it.
Verify that ASC logs now show SecuritySecretsStored is 1

msg="Transformed Metric to SecuritySecretsStored,service=app-new-service counter-count=1 1664992320175487900\n' 

Secure Mode

Generate secure compose with asc-metrics so it adds the service key to the appropriate security services
Edit compose to

  • remove asc-metrics so it doesn't conflict with locally run instance
  • add the app-new-service service key to appropriate security services

Run the customized secure compose file make run up
Run ACS with metrics-influxdb profile from command line with DEBUG logging and security on

WRITABLE_LOGLEVEL=DEBUG EDGEX_SECURITY_SECRET_STORE=true ./app-service-configurable -p=metrics-influxdb | grep Transformed Metric

Run app template service from command-line with DEBUG logging and security on

sudo WRITABLE_LOGLEVEL=DEBUG  DGEX_SECURITY_SECRET_STORE=true ./new-app-service -cp

Verify ever 30 secs it logs the following:

msg="Publish 2 metrics to the 'edgex/telemetry/app-new-service' base topic"
msg="Reported metrics..."

Verify ASC logs have the following:

msg="Transformed Metric to 'SecuritySecretsRequested,service=app-new-service counter-count=1 1664992310177494700\n'
msg="Transformed Metric to SecuritySecretsStored,service=app-new-service counter-count=0 1664992310177494700\n'

Push a new secret into the service SecretStore using the http://localhost:59740/api/v2/secret endpoint

{
   "apiVersion":"v2",
   "path":"MyPath",
   "secretData":[
      {
         "key":"MySecretKey",
         "value":"MySecretValue"
      }
   ]
}

Verify that ASC logs now show SecuritySecretsStored is 1

msg="Transformed Metric to SecuritySecretsStored,service=app-new-service counter-count=1 1664992320175487900\n' 

Core Support service backward compatibility

Build local binary for Support Notifications
Run non-secure stack using compose builder

make run no-secty ds-virtual

Stop Support Notifications container
Run Support Notifications from command-line

WRITABLE_LOGLEVEL=DEBUG  DGEX_SECURITY_SECRET_STORE=false ./support-notifications

Verify service starts properly and has the following log message:

MetricsManager not available. General common service metrics will not be reported. 

New Dependency Instructions (If applicable)

@lenny-goodell lenny-goodell force-pushed the security-secret-metrics branch from ec4be6f to 27a3725 Compare October 5, 2022 19:21
@codecov-commenter
Copy link

Codecov Report

Merging #376 (27a3725) into main (f58aa0b) will decrease coverage by 0.15%.
The diff coverage is 44.44%.

@@            Coverage Diff             @@
##             main     #376      +/-   ##
==========================================
- Coverage   59.02%   58.86%   -0.16%     
==========================================
  Files          24       24              
  Lines        1645     1663      +18     
==========================================
+ Hits          971      979       +8     
- Misses        630      640      +10     
  Partials       44       44              
Impacted Files Coverage Δ
bootstrap/secret/insecure.go 77.38% <44.44%> (-3.96%) ⬇️
bootstrap/secret/secure.go 77.83% <44.44%> (-1.63%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@lenny-goodell lenny-goodell merged commit 42c52e2 into edgexfoundry:main Oct 5, 2022
@lenny-goodell lenny-goodell deleted the security-secret-metrics branch October 5, 2022 21:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants