Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-24743

Generate a service to provide credentials to services

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: efd
    • Labels:
      None

      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.

        Attachments

          Activity

          Hide
          krughoff 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
          krughoff 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
          jsick 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” ?

          Show
          jsick 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
          jsick 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
          jsick 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
          krughoff 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
          krughoff 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:
              krughoff Simon Krughoff
              Reporter:
              krughoff 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:

                Summary Panel