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(snap): add additional tokens for app-service-configurable profiles #3825

Merged
merged 3 commits into from
Nov 29, 2021
Merged

feat(snap): add additional tokens for app-service-configurable profiles #3825

merged 3 commits into from
Nov 29, 2021

Conversation

siggiskulason
Copy link

@siggiskulason siggiskulason commented Nov 24, 2021

The external-mqtt-trigger and app-push-to-core profiles are used by the TAF integration tests.

This PR enables those profiles by creating tokens for them and making the tokens available for
the edgex-app-service-configurable snap.

Signed-off-by: Siggi Skulason [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/edgex-go/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?) n/a

Testing Instructions

  1. Create a clean build of the snap
  2. Install the snap
  3. Confirm the token exists: sudo ls /var/snap/edgexfoundry/current/secrets/app-external-mqtt-trigger/secrets-token.json

New Dependency Instructions (If applicable)

@farshidtz
Copy link
Member

Can't we add this token for the test via the config arguments?
https://github.com/edgexfoundry/edgex-go/tree/main/snap#secret-store-settings-prefix-envsecurity-secret-store

It also applies to app-functional-tests.

@siggiskulason
Copy link
Author

siggiskulason commented Nov 24, 2021

Can't we add this token for the test via the config arguments? https://github.com/edgexfoundry/edgex-go/tree/main/snap#secret-store-settings-prefix-envsecurity-secret-store

It also applies to app-functional-tests.

Yes, indeed, I can do

LIST=`sudo snap get edgexfoundry env.security-secret-store.add-secretstore-tokens`
LIST2=`sudo snap get edgexfoundry env.security-secret-store.add-known-secrets`
sudo snap set edgexfoundry env.security-secret-store.add-secretstore-tokens="$LIST,app-external-mqtt-trigger"
sudo snap set edgexfoundry env.security-secret-store.add-known-secrets="$LIST2,redisdb[app-external-mqtt-trigger]"
sudo snap restart edgexfoundry.security-secretstore-setup 

and that generates the token. However, it adds a few extra steps and the requirement to revoke and regenerate all tokens and since this profile is one of the built-in profiles of app-service-configurable, it might make sense to include a token for it by default.

Also - the config hook in edgexfoundry currently expects all the tokens to be there for the slots listed in the connecting snap. So if we add a slot there for this profile (which PR edgexfoundry/app-service-configurable#361 does), then the content interface doesn't connect unless that token in present. (we get an error like edgex-secretstore-token: could not find token for app-external-mqtt-trigger and the snap doesn't start

@siggiskulason
Copy link
Author

I added a 2nd commit to this PR to add the app-push-to-core token as well, so we now have tokens for all 6 built-in profiles.

Siggi Skulason added 2 commits November 25, 2021 10:20
The external-mqtt-trigger profile is used by the TAF integration tests.
This commit enables that profile by creating a token for that profile and making it available for
the edgex-app-service-configurable snap.

Signed-off-by: Siggi Skulason <[email protected]>
This commit enables the push-to-core app-service-configurable profile
by creating a token for that profile and making it available for
the edgex-app-service-configurable snap.

Signed-off-by: Siggi Skulason <[email protected]>
@siggiskulason
Copy link
Author

force-pushed a rebase to main

Copy link
Member

@farshidtz farshidtz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ignore this..

Copy link
Member

@farshidtz farshidtz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. But please update the PR title:

  • multiple tokens are added now
  • sounds more like a new feature (feat) than fix

@siggiskulason siggiskulason changed the title fix(snap): add token for the external-mqtt-trigger profile feat(snap): add additional tokens for app-service-configurable profiles Nov 29, 2021
@siggiskulason
Copy link
Author

Looks good. But please update the PR title:

  • multiple tokens are added now
  • sounds more like a new feature (feat) than fix

thanks - done.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
9.2% 9.2% Duplication

Copy link
Member

@farshidtz farshidtz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @siggiskulason.

Copy link
Member

@lenny-goodell lenny-goodell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@siggiskulason siggiskulason merged commit 23881e6 into edgexfoundry:main Nov 29, 2021
farshidtz pushed a commit to farshidtz/edgex-go that referenced this pull request Dec 2, 2021
…es (edgexfoundry#3825)

* feat(snap): add additional tokens for app-service-configurable profiles

The external-mqtt-trigger and app-push-to-core profiles are used by the TAF integration tests.

This commit enables those profiles by creating tokens for them and making the tokens available for
the edgex-app-service-configurable snap.

Signed-off-by: Siggi Skulason <[email protected]>
farshidtz pushed a commit to farshidtz/edgex-go that referenced this pull request Dec 2, 2021
…es (edgexfoundry#3825)

* feat(snap): add additional tokens for app-service-configurable profiles

The external-mqtt-trigger and app-push-to-core profiles are used by the TAF integration tests.

This commit enables those profiles by creating tokens for them and making the tokens available for
the edgex-app-service-configurable snap.

Signed-off-by: Siggi Skulason <[email protected]>
(cherry picked from commit 23881e6)
@farshidtz farshidtz added the jakarta dot - 2.1.1 Jakarta Dot Release 2.1.1 label Dec 7, 2021
farshidtz pushed a commit to farshidtz/edgex-go that referenced this pull request Jun 3, 2022
…es (edgexfoundry#3825)

* feat(snap): add additional tokens for app-service-configurable profiles

The external-mqtt-trigger and app-push-to-core profiles are used by the TAF integration tests.

This commit enables those profiles by creating tokens for them and making the tokens available for
the edgex-app-service-configurable snap.

Signed-off-by: Siggi Skulason <[email protected]>
(cherry picked from commit 23881e6)
farshidtz pushed a commit to farshidtz/edgex-go that referenced this pull request Jun 3, 2022
…es (edgexfoundry#3825)

* feat(snap): add additional tokens for app-service-configurable profiles

The external-mqtt-trigger and app-push-to-core profiles are used by the TAF integration tests.

This commit enables those profiles by creating tokens for them and making the tokens available for
the edgex-app-service-configurable snap.

Signed-off-by: Siggi Skulason <[email protected]>
(cherry picked from commit 23881e6)
farshidtz added a commit that referenced this pull request Jun 3, 2022
…es (#3825) (#4043)

* feat(snap): add additional tokens for app-service-configurable profiles

The external-mqtt-trigger and app-push-to-core profiles are used by the TAF integration tests.

This commit enables those profiles by creating tokens for them and making the tokens available for
the edgex-app-service-configurable snap.

Signed-off-by: Siggi Skulason <[email protected]>
(cherry picked from commit 23881e6)

Co-authored-by: Siggi Skulason <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
jakarta dot - 2.1.1 Jakarta Dot Release 2.1.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants