Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use composite template's contributions path as fallback search for template inputs #1221

Conversation

grokspawn
Copy link
Contributor

@grokspawn grokspawn commented Feb 7, 2024

Description of the change:

since using the composite template means the union of requirements from two parties, it can be confusing sometimes to understand the context of resource references.

For example, in the contribution schema olm.composite there are references to input filesystem elements in Components[].Strategy.Template.Config.Input (ref) but these may be relative or absolute references. And if relative... then relative to what?

This PR attempts to address this by fallback searching for input references relative to the cwd of the contribution file (if driven by CLI) or the Components[].Strategy.Template.Config.ContributionPath (if provided).

For example, given a contribution of the format

schema: olm.composite
components:
  - name: catalog-of-wonders
    destination:
      path: my-operator
    strategy:
      name: semver
      template:
        schema: olm.builder.semver
        config:
          input: semver-catalog-template.yaml
          output: catalog.yaml

the template expansion will look for semver-catalog-template.yaml in the CWD of the executed command. But if the file is actually in a catalog-data subdirectory but the command is executed in the parent, then it won't be solved, for e.g.

catalog-data
├── basic-catalog-template.yaml
└── contributions.yaml

The catalogs.yaml file which fulfills the olm.composite.catalogs spec may be local or remote, so it is unsuitable as a search location, but the olm.composite source is always local, which allows us to infer a possible anchor point for relative references.

In case a naiive access attempt for the specified contribution input file fails, we use the resulting anchor in a fallback attempt to resolve input references.

Motivation for the change:

Reviewer Checklist

  • Implementation matches the proposed design, or proposal is updated to match implementation
  • Sufficient unit test coverage
  • Sufficient end-to-end test coverage
  • Docs updated or added to /docs
  • Commit messages sensible and descriptive

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 7, 2024
@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 7, 2024
@grokspawn grokspawn force-pushed the composite-relative-path-input branch from 1264e91 to 0ed37d9 Compare February 7, 2024 22:06
Copy link

codecov bot commented Feb 7, 2024

Codecov Report

Attention: 12 lines in your changes are missing coverage. Please review.

Comparison is base (4c69b4a) 53.79% compared to head (27ea513) 53.81%.

Files Patch % Lines
alpha/template/composite/builder.go 33.33% 9 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1221      +/-   ##
==========================================
+ Coverage   53.79%   53.81%   +0.02%     
==========================================
  Files         108      108              
  Lines       10368    10385      +17     
==========================================
+ Hits         5577     5589      +12     
- Misses       3809     3814       +5     
  Partials      982      982              

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

@grokspawn grokspawn force-pushed the composite-relative-path-input branch 2 times, most recently from a1c4f64 to b29dedc Compare February 9, 2024 19:54
@grokspawn grokspawn changed the title WIP: use contributions path as fallback search for template inputs use composite template's contributions path as fallback search for template inputs Feb 9, 2024
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 9, 2024
@grokspawn grokspawn force-pushed the composite-relative-path-input branch from b29dedc to 74571b1 Compare February 9, 2024 20:05
Signed-off-by: Jordan Keister <jordan@nimblewidget.com>
@grokspawn grokspawn force-pushed the composite-relative-path-input branch from 74571b1 to 27ea513 Compare February 14, 2024 15:17
@grokspawn grokspawn enabled auto-merge February 14, 2024 15:20
@grokspawn grokspawn added this pull request to the merge queue Feb 14, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Feb 14, 2024
Copy link
Contributor

@everettraven everettraven left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 14, 2024
Copy link
Contributor

openshift-ci bot commented Feb 14, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: everettraven, grokspawn

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

The pull request process is described 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

@openshift-merge-bot openshift-merge-bot bot merged commit d924b76 into operator-framework:master Feb 14, 2024
11 of 13 checks passed
@grokspawn grokspawn deleted the composite-relative-path-input branch February 14, 2024 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants