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 #361

Merged
merged 3 commits into from
Dec 6, 2021
Merged

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

merged 3 commits into from
Dec 6, 2021

Conversation

siggiskulason
Copy link

@siggiskulason siggiskulason commented Nov 24, 2021

This PR adds updates the snap so that the secret-store tokens for

  1. external-mqtt-trigger
  2. push-to-core

are copied from the edgexfoundry snap. Both are built-in profiles that are referenced in the TAF tests

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/app-service-configurable/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

To test:

  1. This PR depends on the edgexfoundry also being updated in PR feat(snap): add additional tokens for app-service-configurable profiles edgex-go#3825. Use the edgexfoundry snap from that PR.
  2. Build the edgex-app-service-configurable snap
  3. Install it and then connect the interfaces so that it gets the token: sudo snap connect edgexfoundry:edgex-secretstore-token edgex-app-service-configurable
  4. set the profile:sudo snap set edgex-app-service-configurable profile=external-mqtt-trigger
  5. start the service and confirm in the journal that it has started with the secret-store token

New Dependency Instructions (If applicable)

The external-mqtt-trigger profile is used by the TAF integration tests.
This commit updates the snap so that the secret-store token
for that profile is copied from the edgexfoundry snap

Signed-off-by: Siggi Skulason <[email protected]>
snap/snapcraft.yaml Outdated Show resolved Hide resolved
This commit updates the snap so that the token for the
push-to-core secret-store profile is copied from the edgexfoundry snap

It also changes the list of profiles to a YAML list to shorten the line

Signed-off-by: Siggi Skulason <[email protected]>
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 added now
  • fix or feat?

- $SNAP_DATA/app-external-mqtt-trigger
- $SNAP_DATA/app-functional-tests
- $SNAP_DATA/app-push-to-core
- $SNAP_DATA/app-rules-engine
Copy link
Member

Choose a reason for hiding this comment

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

In future, we need to improve the way this content interface works to allow receiving tokens that are requested dynamically from the secret store, e.g. for development, testing, multiple custom names.

@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 added now
  • fix or feat?

Done

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

@lenny-goodell lenny-goodell merged commit adf35ca into edgexfoundry:main Dec 6, 2021
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