Skip to content

feat: environment variables and secrets#549

Draft
AlexanderLanin wants to merge 4 commits intoeclipse-csi:mainfrom
AlexanderLanin:env_vars
Draft

feat: environment variables and secrets#549
AlexanderLanin wants to merge 4 commits intoeclipse-csi:mainfrom
AlexanderLanin:env_vars

Conversation

@AlexanderLanin
Copy link
Contributor

This is my first extension of otterdog, so chances are I misunderstood some concepts along the way.
If you spot anything odd or flat-out wrong, please point it out — I’m happy to fix things.

Questions / open points

  • Most GitHub API access seems to live in models/github_organization.py, but not consistently.
    Should environment variables & secrets also be retrieved there, or in models/environment.py (as done in a few other cases like repo languages or bypass actors)?

  • parent_repository feels a bit hacky. That said, when applying a LivePatch (e.g. for an environment variable), I need some way to figure out which repository it belongs to. Is there a cleaner / more idiomatic way to do this?

  • I reused newRepoVariable, since a hypothetical newEnvVariable would be 100% identical. Does that match the intended design, or would you prefer separate helpers anyway?

  • I’m not fully confident I handled nested elements correctly for all operations. After a while I honestly lost overview of all the different objects, models, and dicts involved. I might open a parallel PR with some end to end testing.

  • There don’t seem to be many existing tests in this area that I could extend. I ended up adding tests in tests/models/test_environment.py. Is that the right place, or should these live somewhere else?

Resolves #537

@AlexanderLanin
Copy link
Contributor Author

Status: I have a typo in the code, but all tests are passing. Trying to figure out what kind of tests are missing.

@netomi
Copy link
Member

netomi commented Dec 23, 2025

wow that first draft looks promising.
you ask the right questions, in the case of an environment secret you need to know not only the direct parent but also the parent's parent. So far the parent_object was a single object as this was sufficient, but we can extend to a list if needed.

@netomi
Copy link
Member

netomi commented Dec 23, 2025

I would not reuse newRepoVariable, as this is confusing. While it might be 100% identical to that atm, things can change, also it looks weird in the jsonnet file imho.

@AlexanderLanin
Copy link
Contributor Author

Update: while fixing environment secrets, I realized we were missing tests that cover the full flow from model changes to GitHub API calls. (See my previous comment)

That’s why this PR now contains a larger testing part. These tests start from model objects, generate LivePatches as part of the SUT, and verify the resulting HTTP calls. See docstrings for furhter details.

They’re not full E2E tests, but they give good confidence that “anything to model” and “anything from model” works, without the setup and debugging pain of true end-to-end tests.

Happy to split this out if you’d prefer smaller PRs.

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.

Request: Support environment secrets

2 participants