Skip to content

Add environment variables/secrets, repository and org dependabot secrets#614

Open
WolfgangFischerEtas wants to merge 14 commits into
eclipse-csi:mainfrom
etas-contrib:feature/pr_environment
Open

Add environment variables/secrets, repository and org dependabot secrets#614
WolfgangFischerEtas wants to merge 14 commits into
eclipse-csi:mainfrom
etas-contrib:feature/pr_environment

Conversation

@WolfgangFischerEtas

Copy link
Copy Markdown
Contributor

Overview

This pull request extends Otterdog’s model hierarchy and secret management capabilities. It introduces a new environment‑level layer for variables and secrets, adds Dependabot secrets at both repository and organization level, and updates the documentation accordingly.
New Features

  • Environment‑level variables and secrets
    Environments now support their own variables and secrets. This introduces a new hierarchical layer, forming a structured chain of Repository → Environment → Variables/Secrets.

  • Dependabot secrets at organization level
    Organizations can now define and manage Dependabot secrets centrally, including support for redacted values and credential‑provider behavior.

  • Dependabot secrets at repository level
    Repositories now support Dependabot secrets in the same way as other secret types, including import, export, and validation.

Model Changes

Introduction of parent_object
The new environment layer requires each model object to know its position within the hierarchy. To support this, an internal field was added:

    parent_object: ModelObject | None = dataclasses.field(
        default=None,
        kw_only=True,
        repr=False,
        compare=False,
        metadata={"model_only": True},
    )

This field links each object to its parent in the model tree, enabling consistent validation, navigation, and API construction. It is internal only: it is not serialized, not compared, and not part of the external configuration. The model_only: True metadata ensures it remains strictly an internal modeling detail.

Note on the current approach

The introduction of the new hierarchy also exposed a structural inconsistency in ModelObject. The class now contains both a parent_object field and methods that still accept a parent_object parameter, for example:

    def get_model_header(self, parent_object: ModelObject | None = None) -> str:
        header = f"[bold]{self.model_object_name}[/]"

This hybrid approach is not ideal. With the new internal parent_object field (created via dataclasses.field and marked as model_only: True), methods like get_model_header should eventually rely on the stored parent reference instead of receiving it as a parameter. This cleanup is out of scope for this PR but should be addressed in a follow‑up to ensure a consistent and unified model design.

@kairoaraujo kairoaraujo self-requested a review March 12, 2026 14:24
@AlexanderLanin

Copy link
Copy Markdown
Contributor

This closes #537

@kairoaraujo kairoaraujo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hi @WolfgangFischerEtas, thank you for the PR.

Looking the code, I'm just curious if we should worry about that this PR adds 2 new API calls per environment (one for secrets, one for variables).
Which will be 1 call per repo + 2 calls × number of environments

Another performance suggestion: can we use asyncio.gather() for parallel repo processing?

Comment thread otterdog/models/env_secret.py Outdated
Comment thread docs/reference/organization/codespaces_secret.md
Comment thread docs/reference/organization/repository/codespaces_secret.md
Comment thread otterdog/providers/github/rest/env_client.py
@WolfgangFischerEtas

Copy link
Copy Markdown
Contributor Author

I would not worry too much about these 2 additonal calls, they are both encapsulated in

            if jsonnet_config.default_env_secret_config is not None:
                # get secrets of the repo environment
                secrets = await rest_api.env.get_secrets(github_id, repo.name, environment.name)

therefore if you're not interested in these secrets and variables just remove the default....config from the otterdog_defaults.libjsonnet and the additional calls are not made.

I haven't thought about the asyncio.gather function. But if we're doing more calls in parallel are we then getting into the next trouble with the github rate limits?
I saw this comment in github_organization.py:
# limit the number of repos that are processed concurrently to avoid hitting secondary rate limits
and I'm more concerned about the increasing number of concurrent requests than a slightly longer processing time.

@WolfgangFischerEtas

Copy link
Copy Markdown
Contributor Author

@kairoaraujo:
I’ve gone through all review comments and addressed everything that was raised. Please feel free to take another look whenever you have time. Let me know if anything still needs adjustment.

@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
6.4% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@heurtematte

Copy link
Copy Markdown
Contributor

@WolfgangFischerEtas Thanks a lot for the contribution! I’m currently reviewing the code.

One of the first things I noticed is that there is quite a bit of duplicated code in this PR. Would you mind doing some refactoring where possible?

A few areas where improvements could likely be made fairly easily are:

  • rest/env_client.py, especially between update_secret and add_secret
  • the Otterdog models, for example:
    • repo_codespaces_secret / repo_dependabot_secret
    • organization_codespaces_secret / organization_dependabot_secret
    • env_secret / env_variable

There may be other similar cases as well.

@netomi

netomi commented Jul 11, 2026

Copy link
Copy Markdown
Member

in #689 I have now merged in support for env variables and secret. What is missing from the PR are the codespaces and dependabot secret.

I wanted to move things forward and merge in smaller PRs that I can more easily test. I did integrate the to_jsonnet implementation from this PR,

There are a few review comments from Claude for this PR that I wanted to fix but I could not push to the PR so I went ahead with the other PR first, but would like to merge in the remaining stuff from this PR.

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.

5 participants