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

imageproxy: Add a method to get image configuration #21

Merged
merged 1 commit into from Dec 8, 2021

Conversation

cgwalters
Copy link
Collaborator

We need this for a few reasons:

  • It has the layer ordering, which we are currently ignoring
    but will probably be important for correctness later
  • It is the only thing that has extensible metadata that will
    survive round trips through docker schema 2

@cgwalters
Copy link
Collaborator Author

Depends containers/skopeo#1523

Copy link
Collaborator

@jlebon jlebon left a comment

Choose a reason for hiding this comment

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

Looks sane to me.

@cgwalters cgwalters force-pushed the proxy-config branch 2 times, most recently from 2c5eb22 to a639a65 Compare December 8, 2021 14:32
@cgwalters cgwalters marked this pull request as ready for review December 8, 2021 14:32
@cgwalters
Copy link
Collaborator Author

OK, I've updated this 🆕 with infrastructure to support runtime matching of the semver, moving new API methods to a new struct.

We need this for a few reasons:

 - It has the layer ordering, which we are *currently* ignoring
   but will probably be important for correctness later
 - It is the only thing that has extensible metadata that will
   survive round trips through docker schema 2
@cgwalters
Copy link
Collaborator Author

Tested in concert with ostreedev/ostree-rs-ext#176

@cgwalters
Copy link
Collaborator Author

My OCD didn't allow having a runtime unwrap() on the Option, I realized that it's actually more elegant to represent the transient state of "proxy created but haven't fetched semver yet" by representing it as a version of 0.0.0. Then there's no need for a potential runtime panic.

Copy link
Collaborator

@jlebon jlebon left a comment

Choose a reason for hiding this comment

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

LGTM!

@cgwalters cgwalters merged commit 5ea91ae into containers:main Dec 8, 2021

Ok(r)
}

/// If the proxy supports at least semver 0.2.3, return a wrapper with methods from that version.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Interesting approach. I can imagine this becoming unwieldy if we have lots of different versions we need to expose. But I guess clients are coupled enough that we can ensure there's not a large skew to maintain.

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.

None yet

2 participants