# Generate a service to provide credentials to services

XMLWordPrintable

## Details

• Type: Story
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
None
• Story Points:
6
• Epic Link:
• Team:
SQuaRE
• Urgent?:
No

## Description

Currently the EFD helper classes prefer to find credentials in a file in a known location. We will keep that mechanism, but will also provide a service that can serve low risk (read only) credentials. This will allow us to transparently update the credentials without the utilities that depend on them update a file on disk at a know location. The service will run on roundtable and will serve the credentials from a k8s secret provided by Vault Secrets Operator.

## Activity

Hide
Simon Krughoff added a comment -

This service is now called segwarides. I've been putting commits on master, so the diff from the templates is not straightforward (nor is a PR). I'm interested to know what I should do.

Show
Simon Krughoff added a comment - This service is now called segwarides . I've been putting commits on master, so the diff from the templates is not straightforward (nor is a PR). I'm interested to know what I should do.
Hide
Jonathan Sick added a comment -

app.py

 root_app["segwarides/creds_getter"] = get_credentials_by_key root_app["segwarides/creds_mapper_maker"] = make_credential_map 

I think the normal thing here would be for code that uses these functions to just import them and use them. The app is mostly for holding configuration or for holding singleton instances of things that are initialized on startup.

For example, you might refactor make_credential_map and get_credentials_by_key into a class. Then on app start up you’d initialize the credential map and add it to your app.

config.py

You’re casting the default Path to a str. Consider instead using pathlib.Path natively throughout the app. For example, you can read the contents of a file with the Path.read_text() method. Path is really awesome!

https://github.com/lsst-sqre/segwarides/commit/0f06537ffe15e8ead1054c74487b568eeeeb724f

This code is a bit tricky. Normally I’d try to raise errors as soon as possible on app start up so its known, but I guess here credentials are being loaded more dynamically and you only want the error to pop up as a user requests it.

You might want to add a comment to the make_credential_map function along the same lines as the git commit comment so the reasoning here doesn’t get buried.

README.rst

 This is an Rubin Observatory DM SQuaRE api.lsst.codes microservice, developed with the Safir __ framework. 

Technically it’ll be served from roundtable.lsst.io. Maybe say:

 This is a Rubin Observatory DM SQuaRE microservice, developed with the Safir __ framework and hosted on Roundtable __. 

You can delete the line saying: “Get started with development with the tutorial <https://safir.lsst.io/set-up-from-template.html>__.”

  - github.com/lsst-sqre/segwarides git//manifests/base?ref=0.3.0 

I think there’s a typo here? Maybe it should be “segwarides.git” ?

Show
Jonathan Sick added a comment - app.py root_app["segwarides/creds_getter"] = get_credentials_by_key root_app["segwarides/creds_mapper_maker"] = make_credential_map I think the normal thing here would be for code that uses these functions to just import them and use them. The app is mostly for holding configuration or for holding singleton instances of things that are initialized on startup. For example, you might refactor make_credential_map and get_credentials_by_key into a class. Then on app start up you’d initialize the credential map and add it to your app. config.py You’re casting the default Path to a str. Consider instead using pathlib.Path natively throughout the app. For example, you can read the contents of a file with the Path.read_text() method. Path is really awesome! https://github.com/lsst-sqre/segwarides/commit/0f06537ffe15e8ead1054c74487b568eeeeb724f This code is a bit tricky. Normally I’d try to raise errors as soon as possible on app start up so its known, but I guess here credentials are being loaded more dynamically and you only want the error to pop up as a user requests it. You might want to add a comment to the make_credential_map function along the same lines as the git commit comment so the reasoning here doesn’t get buried. README.rst This is an Rubin Observatory DM SQuaRE api.lsst.codes microservice, developed with the Safir <https://safir.lsst.io>__ framework. Technically it’ll be served from roundtable.lsst.io. Maybe say: This is a Rubin Observatory DM SQuaRE microservice, developed with the Safir <https://safir.lsst.io>__ framework and hosted on Roundtable <https://roundtable.lsst.io>__. You can delete the line saying: “Get started with development with the tutorial < https://safir.lsst.io/set-up-from-template.html >__.” - github.com/lsst-sqre/segwarides git//manifests/base?ref=0.3.0 I think there’s a typo here? Maybe it should be “segwarides.git” ?
Hide
Jonathan Sick added a comment -

This looks very good. I think the main thing to do in the long run is to consider refactoring the mapper data and functions into a class and putting an instance of that class on the app. I think that'll be easier to expand and maintain in the future.

Show
Jonathan Sick added a comment - This looks very good. I think the main thing to do in the long run is to consider refactoring the mapper data and functions into a class and putting an instance of that class on the app. I think that'll be easier to expand and maintain in the future.
Hide
Simon Krughoff added a comment -

Jonathan Sick I took your advice and refactored into a class. Please let me know if you think my approach is what you had in mind. Since it is a reasonably big change, I'd like to get one more pass on just the mapper class before I merge the other PRs.

Show
Simon Krughoff added a comment - Jonathan Sick I took your advice and refactored into a class. Please let me know if you think my approach is what you had in mind. Since it is a reasonably big change, I'd like to get one more pass on just the mapper class before I merge the other PRs.

## People

• Assignee:
Simon Krughoff
Reporter:
Simon Krughoff
Reviewers:
Jonathan Sick
Watchers:
Jonathan Sick, Simon Krughoff
• Votes:
0 Vote for this issue
Watchers:
2 Start watching this issue

## Dates

• Created:
Updated:
Resolved: