Skip to content

Conversation

@dheerajodha
Copy link
Contributor

@dheerajodha dheerajodha commented Jun 9, 2025

  • This commit enhances the ociDescriptor function
    current approach to check OCI image URIs that are
    pinned by tag, as it just checks for presence of @

  • So, now we make use of standard functions from the
    "go-containerregistry" package

resolves: EC-1312

},
},
},
{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't add test coverage for the happy path because the entry above already covers it.

@codecov
Copy link

codecov bot commented Jun 9, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.85%. Comparing base (ef350b8) to head (d228309).
Report is 4 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2568      +/-   ##
==========================================
+ Coverage   68.73%   68.85%   +0.11%     
==========================================
  Files         101      101              
  Lines        8557     8569      +12     
==========================================
+ Hits         5882     5900      +18     
+ Misses       2675     2669       -6     
Flag Coverage Δ
generative 68.85% <100.00%> (+0.11%) ⬆️
integration 68.85% <100.00%> (+0.11%) ⬆️
unit 68.85% <100.00%> (+0.11%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
internal/rego/oci/oci.go 95.12% <100.00%> (+1.09%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dheerajodha dheerajodha force-pushed the EC-1312 branch 2 times, most recently from 4945683 to d228309 Compare June 10, 2025 14:03
@dheerajodha
Copy link
Contributor Author

/retest

@dheerajodha
Copy link
Contributor Author

The build and test PLRs are stuck with message Expected — Waiting for status to be reported, and /retest didin't work, I'll push a commit to retrigger them

@dheerajodha
Copy link
Contributor Author

still seeing the same status :/

@joejstuart
Copy link
Contributor

Doesn't the resolveIfNeeded function on line 392 ensure uri contains a digest?

uri, err := resolveIfNeeded(client, string(uriValue))
if err != nil {
    logger.WithField("action", "resolveIfNeeded").Error(err)
    return nil, nil
}

@dheerajodha
Copy link
Contributor Author

Doesn't the resolveIfNeeded function on line 392 ensure uri contains a digest?

Thanks! that's true, added a reply here in the Slack thread. resolveIfNeeded does resolve tag to a digest, but I still feel it can be improved by using standard library functions following OCI ref specs, especially here:

if !strings.Contains(uri, "@") {

Added the changes in the latest commit.

@simonbaird
Copy link
Member

I gave it a little test like this:

$ make build && ./dist/ec --debug opa eval 'ec.oci.descriptor("registry.access.redhat.com/ubi9/python-39:latest")'

* This commit enhances the `ociDescriptor` function
  current approach to check OCI image URIs that are
  pinned by tag, as it just checks for presence of `@`

* So, now we make use of functions from the
  "go-containerregistry" package

resolves: EC-1312
@dheerajodha dheerajodha changed the title feat: Support tag-based OCI refs in ociDescriptor improve OCI ref Parsing and Digest Resolution Jun 17, 2025
@dheerajodha
Copy link
Contributor Author

@simonbaird or @joejstuart can you please merge this PR whenever you can? (I don't have the rights)
Thank you for your help and patience on this one!

@joejstuart joejstuart merged commit e9225f9 into conforma:main Jun 23, 2025
13 checks passed
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.

4 participants