Skip to content

Conversation

@deadlycoconuts
Copy link
Contributor

@deadlycoconuts deadlycoconuts commented Feb 14, 2025

Context

Similar to what has been done for Merlin in caraml-dev/merlin#627 and caraml-dev/merlin#628, this PR introduces a new feature to mount secrets stored in MLP into their Turing router deployments (enrichers and docker/pyfunc ensemblers only) and batch ensembling workflows as environment variables. This is similar to the existing feature whereby users are able to mount their Google Cloud Platform's service account key stored in MLP as a secret within their enrichers, docker/pyfunc ensemblers and batch ensembling workflows.

The changes in this PR covers all the necessary changes required in the API, SDK and the UI.

API

The changes here are relatively straightforward, and involve 2 main workflows:

  • retrieving secrets from MLP, and
  • mounting them as environment variables into the enricher, docker/pyfunc ensembler and/or Spark driver/executor spec (for batch ensembling)

Some refactoring has been done (not perfect) to streamline the secret retrieval and mounting steps with the existing service account secret key retrieval and environment variables mounting workflows. These changes are relatively evident in this PR.

The DB also requires some migration to work, and there are updates to the enricher and ensembler_configs tables to create an empty list [] for the new secrets field (this is consistent with how the env field (for environment variables) are also expected to exist in those tables even if its value is an empty list.

SDK

The changes here are also pretty straightforward, and also involve adding a new field to the enricher, docker/pyfunc ensembler and ensembling job config classes.

UI

The changes for the UI are similar to what had been done for Merlin, and basically introduces new components for the display of secrets and the configuring of secrets.

Router Creation/Edit Form

Screenshot 2025-02-19 at 6 27 24 PM * Note that there is no existing way to create ensembling jobs via the UI.

Config View for Turing Router Deployment (Enricher and Docker/Pyfunc Ensembler)

Screenshot 2025-02-19 at 6 17 42 PM Screenshot 2025-02-19 at 6 17 54 PM

Config View for Batch Ensembling Job

Screenshot 2025-02-19 at 6 27 48 PM

Additional UI Change

I've updated the react-lazylog dependency from a git source to one that's published on npm. For some reason, installing that dependency from a git source causes yarn to fail because an underlying tool spawn fails on M1 machines.

One way to go around this is to run yarn in a Docker container during development but to save everyone's sanity, I've decided to just publish https://github.com/gojekfarm/react-lazylog#master on npm as a full package instead. This is fine because that fork (of the original react-lazylog) hasn't been updated for ages anyway and the original package has already been archived and read-only since September 2024.

The reason why we're using this fork is supposedly because it contains some custom emitter object - see this and this that the Turing UI uses, and since none of these sources get updated anymore, it's safe to just publish it and use it without fear of dependency issues creeping.

This change also effectively re-reverts the change in this old PR here #386.

Main Modifications

  • api/api/specs/common.yaml - Addition of a new MountedMLPSecret schema to OpenAPI specs
  • api/db-migrations/000016_add_secrets_columns.*.sql - Addition of migration scripts needed to support the new changes
  • api/turing/api/deployment_controller.go - Refactoring of steps needed to retrieve secrets from MLP for enrichers and docker/pyfunc ensemblers
  • api/turing/batch/ensembling/controller.go - Refactoring of steps needed to retrieve secrets from MLP for batch ensembling jobs
  • api/turing/cluster/servicebuilder/service_builder.go - Refactoring of steps needed to mount MLP secrets as environment variables for enrichers and docker/pyfunc ensemblers
  • api/turing/cluster/spark.go - Refactoring of steps needed to mount MLP secrets as environment variables for batch ensembling jobs
  • sdk/turing/batch/config/config.py - Addition of the secrets field to the EnsemblingJobConfig class
  • sdk/turing/mounted_mlp_secret.py - Addition of a new MountedMLPSecret class to represent MLP secrets to be mounted
  • sdk/turing/router/config/enricher.py - Addition of the secrets field to the Enricher class
  • sdk/turing/router/config/router_ensembler_config.py - Addition of the secrets field to the docker/pyfunc ensembler classes
  • ui/package.json - Replacement of the existing react-lazylog package with the new react-lazylog-with-emitter package
  • ui/src/router/components/configuration/components/docker_config_section/SecretsConfigTable.js - Addition of a new panel to configure secrets
  • ui/src/router/components/form/components/docker_config/SecretsPanel.js - Addition of a new panel to display secrets

@deadlycoconuts deadlycoconuts added the enhancement New feature or request label Feb 14, 2025
@deadlycoconuts deadlycoconuts self-assigned this Feb 14, 2025
@codecov
Copy link

codecov bot commented Feb 14, 2025

Codecov Report

Attention: Patch coverage is 97.01493% with 2 lines in your changes missing coverage. Please review.

Project coverage is 95.94%. Comparing base (100c478) to head (c13bb6c).
Report is 9 commits behind head on main.

Files with missing lines Patch % Lines
sdk/turing/router/config/enricher.py 86.66% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #403      +/-   ##
==========================================
+ Coverage   95.92%   95.94%   +0.02%     
==========================================
  Files          25       26       +1     
  Lines        2035     2098      +63     
==========================================
+ Hits         1952     2013      +61     
- Misses         83       85       +2     
Flag Coverage Δ
sdk-test-3.10 95.94% <97.01%> (+0.02%) ⬆️
sdk-test-3.8 95.94% <97.01%> (+0.02%) ⬆️
sdk-test-3.9 95.94% <97.01%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@deadlycoconuts deadlycoconuts changed the title feat(api): Add user-configurable secrets for deployments feat(api): Add user-configurable secrets for deployments and ensembling jobs Feb 19, 2025
@deadlycoconuts deadlycoconuts marked this pull request as ready for review February 19, 2025 11:39
Copy link
Contributor

@bthari bthari left a comment

Choose a reason for hiding this comment

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

Thank you for adding the capability of user configurable secret to Turing and updating the UI as well. I have some little questions but the rest is LGTM! 🚀

@deadlycoconuts
Copy link
Contributor Author

Merging these changes now! Thanks a lot for the quick review @bthari !

@deadlycoconuts deadlycoconuts merged commit 9b0ccd2 into caraml-dev:main Feb 21, 2025
29 checks passed
@deadlycoconuts deadlycoconuts changed the title feat(api): Add user-configurable secrets for deployments and ensembling jobs feat(api,sdk,ui): Add user-configurable secrets for deployments and ensembling jobs Mar 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants