Skip to content

CName.load_from_release_file() enforces pre-parsed cname consistency instead of treating the release file as authoritative #302

Closed
gardenlinux/gardenlinux
#4279

Description

@nkraetzschmar

Summary

The current flow requires constructing a CName from a cname string first and only then loading a .release file, with the loader enforcing consistency between the pre-parsed cname and the release metadata.

This approach is inherently broken for the intended use case: loading builder-generated release metadata.

The release file entries output by the builder are the canonical source of truth for the S3 uploaded metadata. Any cname parsing performed by the library is necessarily heuristic and must not be allowed to override or invalidate builder output.

Core problem

The current flow is effectively:

  1. Parse a cname string using regex and implicit rules.
  2. Load a release file.
  3. Reject the release file if it does not match the previously inferred state.

This inverts authority. A derived interpretation of a cname string is treated as stronger than the metadata emitted by the builder.

This fails as soon as:

  • The filename or caller-supplied cname differs from GARDENLINUX_CNAME
  • The builder formats fields differently than the cname string parser expects

Concrete failure in the shown example

In upload_from_directory, the CName object is constructed from the artifact directory name / filename:

openstack-metal-arm64-today-local.release

This results in a pre-parsed cname that includes the commit (-local).

The release file itself, however, contains:

GARDENLINUX_CNAME=openstack-metal-arm64-today

When load_from_release_file() is called, it compares the release file’s GARDENLINUX_CNAME (parsed into a new CName instance) against the already-initialized self and fails the consistency check.

This is not an edge case. It is a direct consequence of requiring the release file to conform to a cname that was inferred earlier from the filename.

Design issue (main point)

Parsing a cname string first and then requiring the release file to match that parse is fundamentally flawed.

The release file entries output by the builder are the canonical source of truth for the S3 uploaded metadata.
Regex-based cname parsing in the library is secondary and must never veto or reinterpret builder metadata.

The loader should populate fields from the release file directly, without attempting to validate them against a previously parsed cname.

Additional FYI

  • There is a small typo/inconsistency around _feature_platform_cached vs _feature_platforms_cached, which can lead to attribute errors before a release file is loaded.

Expected direction

  • Allow constructing a CName directly from a release file, without requiring an initial cname string.
  • Treat the release file as authoritative and load its fields verbatim.
  • If validation exists, it should only check internal consistency within the release file itself, not against caller-supplied or heuristically parsed input.

This aligns the implementation with how the metadata is produced and should be consumed in practice.

Activity

  1. added this to the 2026-02 milestone on Feb 4, 2026
  2. NotTheEvilOne commented on Feb 4, 2026

    @NotTheEvilOne
    Contributor

    While #305 allows support for broken release files I do not agree with the problem description of the original author:

    • If for an existing Garden Linux canonical name instance additional (cache) data is loaded from a file consistency checks as far as feasible are absolutely required. Otherwise any unrelated file could overwrite e.g. features while something totally different was expected based on a cname given.
    • A new method CName.new_from_release_file() was added to create new instances without a given Garden Linux canonical name.
    • The cname openstack-metal-arm64-today misses the commit ID (local) and is therefore not valid. A commit ID is the mandatory final part of a Garden Linux canonical name. Otherwise no distinction is possible to correctly interpret what a flag, an architecture, a version or a commit ID would be.
    • As the *.release file fortunately contains commit hashes (or local if build without a hash) we can workaround current files not conforming to the expected format.
  3. self-assigned this
    on Feb 4, 2026
  4. nkraetzschmar commented on Feb 5, 2026

    @nkraetzschmar
    ContributorAuthor

    The cname openstack-metal-arm64-today misses the commit ID (local) and is therefore not valid. A commit ID is the mandatory final part of a Garden Linux canonical name. Otherwise no distinction is possible to correctly interpret what a flag, an architecture, a version or a commit ID would be.

    I do not agree. The release file of (e.g. aws 1877.10) looks as follows:

    ID=gardenlinux
    ID_LIKE=debian
    NAME="Garden Linux"
    PRETTY_NAME="Garden Linux 1877.10"
    IMAGE_VERSION=1877.10
    VARIANT_ID=aws-gardener_prod-amd64
    HOME_URL="https://gardenlinux.io"
    SUPPORT_URL="https://github.com/gardenlinux/gardenlinux"
    BUG_REPORT_URL="https://github.com/gardenlinux/gardenlinux/issues"
    GARDENLINUX_CNAME=aws-gardener_prod-amd64-1877.10
    GARDENLINUX_PLATFORM=aws
    GARDENLINUX_FEATURES=log,sap,ssh,_legacy,_nopkg,_prod,_slim,base,server,cloud,aws,multipath,iscsi,nvme,gardener
    GARDENLINUX_VERSION=1877.10
    GARDENLINUX_COMMIT_ID=3d56ba62
    GARDENLINUX_COMMIT_ID_LONG=3d56ba627f7f471e64f36608ffbc84a441d309d9
    

    I would argue whatever format our /etc/os-release specifies as the cname is the authoritative source.

    Also the way the builder accepts cnames as its build target has always been in this format as this is the part one can choose/configure. The commit is added based on the current repo state, not something one can select. Trying to build a cname openstack-metal-arm64-today-local results in:

    $ ./build openstack-metal-arm64-today-local
    Traceback (most recent call last):
      File "/builder/./parse_features", line 223, in <module>
        main()
        ~~~~^^
      File "/builder/./parse_features", line 82, in main
        assert feature in feature_graph.nodes(), f"feature {feature} not found"
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    AssertionError: feature local not found
    Traceback (most recent call last):
      File "/builder/./parse_features", line 223, in <module>
        main()
        ~~~~^^
      File "/builder/./parse_features", line 82, in main
        assert feature in feature_graph.nodes(), f"feature {feature} not found"
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    AssertionError: feature arm64 not found
    make: *** No rule to make target 'openstack-metal-arm64-today-local'.  Stop.
    

    Which make sense, as the builder should only accept as target input things it can choose between. The builder obviously is unable, or at least should not, pick a commit based on what input target was given on the command line.

    So clearly there is some misalignment here in what the python lib considers a cname and what the builder does.

    I'm not saying we have to keep this definition of a cname, but if we want to change the definition of a cname, then this must be a proper design decision / ADR in the main gardenlinux repo and discussed with the team.

    The way the pythonlib parses / understands a cname is not automatically authorative. The gardenlinux repo and the builder are the sources of truth, the pythonlib is intended to support/assist this.

    Any deviation by the lib compared to the behaviour of the gardenlinux image builds is, in my opinion, at first to be considered a bug. If any alternative behaviour is desired this must be properly agreed upon and not implicitly decided by the pythonlib.

  5. nkraetzschmar commented on Feb 5, 2026

    @nkraetzschmar
    ContributorAuthor

    But changing the definition of the cname would also imply breaking changes to e.g. the in-place updates tool, as this also relies on this expected behaviour of what is considered a valid cname: https://github.com/gardenlinux/package-gardenlinux-update/blob/fa1666f7bb715976b2de3e09514efc215a30f25d/src/main.go#L55-L75

  6. added a commit that references this issue on Feb 5, 2026
    d145fdb
  7. NotTheEvilOne commented on Feb 5, 2026

    @NotTheEvilOne
    Contributor

    We need to reconsider this in a broader scale as e.g. with gardenlinux/builder#115 the above call to ./build is accepted and generates the expected output. Until a solution is found for the misalignment between our tooling I added a workaround in #306.

    Furthermore even the builder internally appends $COMMIT at various places to generate a cname as e.g. in builder/make_get_image_dependencies.

  8. added 4 commits that reference this issue on Feb 5, 2026
    c8664d2
    2e61e56
    aac30a6
    03c1db8
  9. nkraetzschmar commented on Feb 5, 2026

    @nkraetzschmar
    ContributorAuthor

    Furthermore even the builder internally appends $COMMIT at various places to generate a cname as e.g. in builder/make_get_image_dependencies.

    Yep, it's added to find the artifact_base that is NOT a cname.

  10. nkraetzschmar commented on Feb 5, 2026

    @nkraetzschmar
    ContributorAuthor

    with gardenlinux/builder#115 the above call to ./build is accepted

    added some comments in that PR on why allowing this call is a bad idea and could result in inconsistent state

  11. NotTheEvilOne commented on Feb 5, 2026

    @NotTheEvilOne
    Contributor

    An ADR has been created as suggested by @nkraetzschmar.

    Regarding the artifact_base discussion the next line refers to the cname. I think we agree that an ADR is required to catch and prevent such corner cases. :)

    artifact_base="${1%"-$COMMIT$extension"}"
    cname="$(basename "$artifact_base")"
    
  12. nkraetzschmar commented on Feb 5, 2026

    @nkraetzschmar
    ContributorAuthor

    Here is my proposal on how I would see the interface between the builder output and what should be uploaded to S3 for GLCI to consume: gardenlinux/gardenlinux#4268

    This is largely independent of if the cname does or does not contain the commit, as for this proposal the publishing logic should be entirely oblivious to what a cname is and how it is structured. All metadata needed to publish can be directly extracted from build artifacts => Even future changes in what is considered a cname or similar would then only require changes in one place (the image build), and would automagically work in the publishing phase.

  13. added 6 commits that reference this issue on Feb 6, 2026
    0717508
    ae5d482
    f68a3c0
    9013dcc
    68c1f3f
    c1d743c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

enhancementEnhancement, improvement, extension

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions