Skip to content

Conversation

@tarilabs
Copy link

@tarilabs tarilabs commented Sep 17, 2025

@ruivieira can you kindly add the labels for me please?
I can't

Summary by Sourcery

Add end-to-end support for uploading evaluation results to OCI registries, including API, CRD, controller, driver, validation, CLI flag, and extensive tests

New Features:

  • Introduce OCI output support in LMEvalJob via a new OCISpec API type and CRD schema additions
  • Extend CreatePod and generateCmd to configure OCI environment variables, volume mounts, and add an --upload-to-oci flag
  • Add UploadToOCI option in driver and implement uploadToOCI method to invoke an OCI upload script on job completion
  • Add command-line flag upload-to-oci in the driver binary to enable OCI uploads

Enhancements:

  • Add helper methods for OCI spec presence, authentication checks, and certificate detection
  • Implement ValidateOCIPath and ValidateOCIAuth functions and integrate them into user input validation

Documentation:

  • Update CRD base manifest to include OCI output properties and validation patterns

Tests:

  • Add comprehensive unit tests for OCI command generation and pod configuration in the controller
  • Add validation tests for OCI helper methods, path validation, and authentication rules
  • Add driver tests covering successful and failed OCI upload scenarios and the uploadToOCI function

@openshift-ci
Copy link

openshift-ci bot commented Sep 17, 2025

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@sourcery-ai
Copy link
Contributor

sourcery-ai bot commented Sep 17, 2025

Reviewer's Guide

This PR implements end-to-end OCI registry support for LMEvalJobs by extending the API, CRD schema, validation, controller, driver, CLI flags, and tests to handle OCI outputs. It introduces a new OCISpec type and helper methods, integrates OCI path/auth validation, augments command generation and pod creation with OCI environment variables, secrets, and certificate volumes, adds a post-run uploadToOCI step in the driver, and includes extensive unit tests covering all OCI scenarios.

Sequence diagram for OCI output environment variable and volume setup in pod creation

sequenceDiagram
    participant Controller
    participant PodSpec
    participant Secret
    participant ConfigMap
    Controller->PodSpec: Check job.Spec.HasOCIOutput()
    alt OCI output configured
        Controller->PodSpec: Add OCI env vars (OCI_REGISTRY, OCI_REPOSITORY, OCI_PATH, ...)
        Controller->Secret: Reference registry/repository/auth secrets
        Controller->PodSpec: Add volume for certificates if CABundle specified
        Controller->ConfigMap: Reference CABundle configmap
        Controller->PodSpec: Mount certificate volume
    end
Loading

Sequence diagram for driver uploading results to OCI after job completion

sequenceDiagram
    participant Driver
    participant OCI Registry
    Driver->Driver: Check Option.UploadToOCI
    alt UploadToOCI is true
        Driver->OCI Registry: Run oci.py with registry and results path
        OCI Registry->Driver: Accept results artifact
    end
Loading

Entity relationship diagram for new OCISpec in Outputs

erDiagram
    Outputs {
        PersistentVolumeClaimManaged
        OCISpec
    }
    OCISpec {
        Registry SecretKeySelector
        Repository SecretKeySelector
        Tag string
        Path string
        Subject string
        UsernameRef SecretKeySelector
        PasswordRef SecretKeySelector
        TokenRef SecretKeySelector
        VerifySSL bool
        CABundle SecretKeySelector
    }
    Outputs ||--|{ OCISpec : contains
    OCISpec }|--|| SecretKeySelector : "uses for registry/repository/auth/cert"
Loading

Class diagram for new and updated types supporting OCI output

classDiagram
    class Outputs {
        +PersistentVolumeClaimManaged pvcManaged
        +OCISpec oci
        +HasExistingPVC() bool
        +HasOCI() bool
    }
    class OCISpec {
        +SecretKeySelector Registry
        +SecretKeySelector Repository
        +string Tag
        +string Path
        +string Subject
        +SecretKeySelector UsernameRef
        +SecretKeySelector PasswordRef
        +SecretKeySelector TokenRef
        +bool VerifySSL
        +SecretKeySelector CABundle
        +HasCertificates() bool
        +HasUsernamePassword() bool
        +HasToken() bool
    }
    class LMEvalJobSpec {
        +Outputs Outputs
        +HasOCIOutput() bool
        +HasCustomOutput() bool
    }
    Outputs o-- OCISpec : "oci"
    OCISpec o-- SecretKeySelector : "Registry/Repository/UsernameRef/PasswordRef/TokenRef/CABundle"
    LMEvalJobSpec o-- Outputs : "outputs"
Loading

File-Level Changes

Change Details Files
Define and integrate OCISpec into the API and CRD
  • Added OCISpec struct with fields for registry, repository, tag, path, subject, auth, SSL, certificates
  • Extended Outputs and LMEvalJobSpec with OCISpec and helper methods (HasOCIOutput, Outputs.HasOCI)
  • Updated deep-copy logic and CRD YAML to include OCI fields
lmevaljob_types.go
zz_generated.deepcopy.go
trustyai.opendatahub.io_lmevaljobs.yaml
Implement OCI path and authentication validation
  • Created ValidateOCIPath to enforce allowed characters and forbid traversal/metacharacters
  • Created ValidateOCIAuth to require exclusive username/password or token
  • Hooked OCI validation into ValidateUserInput
validation.go
lmevaljob_controller_validation_test.go
Extend controller generateCmd and CreatePod for OCI
  • Appended --upload-to-oci flag based on HasOCIOutput
  • Injected OCI env vars (registry, repository, path, tag, subject, auth, SSL) into pod spec
  • Configured certificate volume mounts and env var when CABundle present
lmevaljob_controller.go
lmevaljob_controller_test.go
Add driver uploadToOCI functionality
  • Introduced upload-to-oci CLI flag and DriverOption field
  • Implemented driverImpl.uploadToOCI to invoke OCI script, read env vars, report errors
  • Hooked uploadToOCI call after job completion
driver.go
main.go
driver_test.go
Expand unit tests to cover OCI scenarios
  • Added tests for command flag inclusion and pod environment setup
  • Wrote validation tests for OCI path, auth permutations
  • Added driver tests for successful, disabled, missing-registry, script-failure cases
lmevaljob_controller_test.go
lmevaljob_controller_validation_test.go
driver_test.go

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@openshift-ci
Copy link

openshift-ci bot commented Sep 17, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@tarilabs
Copy link
Author

/ok-to-test
/lgtm

@openshift-ci
Copy link

openshift-ci bot commented Sep 17, 2025

@tarilabs: you cannot LGTM your own PR.

In response to this:

/ok-to-test
/lgtm

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@github-actions
Copy link

github-actions bot commented Sep 17, 2025

PR image build and manifest generation completed successfully!

📦 PR image: quay.io/trustyai/trustyai-service-operator:2273bbae63bc1dc6edaad9208bb5c1d11a2f40e9

📦 LMES driver image: quay.io/trustyai/ta-lmes-driver-ci:2273bbae63bc1dc6edaad9208bb5c1d11a2f40e9

📦 LMES job image: quay.io/trustyai/ta-lmes-job-ci:2273bbae63bc1dc6edaad9208bb5c1d11a2f40e9

📦 Guardrails orchestrator image: quay.io/trustyai/ta-guardrails-orchestrator:latest

🗂️ CI manifests

      devFlags:
        manifests:
          - contextDir: config
            sourcePath: ''
            uri: https://api.github.com/repos/trustyai-explainability/trustyai-service-operator-ci/tarball/operator-2273bbae63bc1dc6edaad9208bb5c1d11a2f40e9

Signed-off-by: tarilabs <[email protected]>
@RobGeada RobGeada added the needs-lmes-build Pull requests that need a new build of the LMES driver and job images for CI label Sep 17, 2025
@openshift-merge-robot
Copy link
Collaborator

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/work-in-progress needs-lmes-build Pull requests that need a new build of the LMES driver and job images for CI needs-rebase ok-to-test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants