xiekeyang / oci-discovery

Contain the OCI Ref-engine Discovery specification and related specifications as an extention to the image specification.
Other
2 stars 1 forks source link

oci_discovery/uri_template: Stub for URI Template expansion #7

Closed wking closed 7 years ago

wking commented 7 years ago

Python's .format() can handle things like {host}, but there's a lot more than that in RFC 6570. Instead of rolling our own implementation, punt to the uritemplate package. The license is:

BSD-3-Clause OR Apache-2.0

which should be compatible with the OCI's Apache-2.0 requirement, even if that requirement applies to third-party dependencies, which is not clear to me.

xiekeyang commented 7 years ago

@wking , It seems this restriction will reject hostname of {IP}:{PORT}, which is too generally used to build local server. How can I discovery image when I set nginx server localhost:8080? I think in this stage we needn't set too much restriction.

And, the OCI index fetching workflow prompts log:

DEBUG:oci_discovery.ref_engine.oci_index_template:fetching an OCI index for 127.0.0.1:8080/app#1.0 from https://127.0.0.1:8080/oci-index/app
WARNING:oci_discovery.ref_engine_discovery:<urlopen error [SSL: UNKNOWN_PROTOCOL] unknown protocol (_ssl.c:645)>
DEBUG:oci_discovery.ref_engine_discovery:discovering ref engines via http://0.0.1:8080/.well-known/oci-host-ref-engines
WARNING:oci_discovery.ref_engine_discovery:failed to fetch http://0.0.1:8080/.well-known/oci-host-ref-engines (HTTP Error 403: Proxy_WarningPage)
DEBUG:oci_discovery.ref_engine_discovery:discovering ref engines via http://0.1:8080/.well-known/oci-host-ref-engines
WARNING:oci_discovery.ref_engine_discovery:failed to fetch http://0.1:8080/.well-known/oci-host-ref-engines (HTTP Error 403: Proxy_WarningPage)
ERROR:root:no Merkle root found for '127.0.0.1:8080/app#1.0'
{}

Actually I can fetch the index:

$ curl http://127.0.0.1:8080/oci-index/app
{
  "schemaVersion": 2,
  "manifests": [
    {
      "mediaType": "application/vnd.oci.image.manifest.v1+json",
      "size": 799,
      "digest": "sha256:e9770a03fbdccdd4632895151a93f9af58bbe2c91fdfaaf73160648d250e6ec3",
      "platform": {
        "architecture": "ppc64le",
        "os": "linux"
      },
      "annotations": {
        "org.opencontainers.image.ref.name": "1.0"
      },
      "casEngines": [
        {
          "protocol": "oci-cas-template-v1",
          "uri": "https://example.com/oci-cas/{algorithm}/{encoded:2}/{encoded}"
        }
      ]
    }
  ]
}

You are split the URI by . one by one. I know you want to resolve the template a.b.c.example.com. But I think it is some complex and not fit to IP address. Could we just check uri once in argument, and postpone the advanced resolution to later?

Now I'm debugging the workflow in README, and is confused a little by some internal mechanism.

wking commented 7 years ago

It seems this restriction will reject hostname of {IP}:{PORT}, which is too generally used to build local server.

I've spun that off into #9, since it relates to the host-based image name spec, while this PR is just about the oci-index-template-v1 implementation.

How can I discovery image when I set nginx server localhost:8080?

I floated some ideas in this comment, although none of them are trivial. However, the _IP_V4_REGEXP patch I suggest there (as a local-testing workaround) is only touching one line, so while it's unfortunate to have to touch code at all for your local testing, it's a pretty small touch.

wking commented 7 years ago

You are split the URI by . one by one. I know you want to resolve the template a.b.c.example.com. But I think it is some complex and not fit to IP address.

It is guarded against IP addresses (here). It's just not guarded against IPv4-address-based-authorities (without the _IP_V4_REGEXP patch suggested here). But since the host-based image name spec requires hosts and not authoritys, that's not a problem with the ancestor walker; it's a problem with the overly-liberal host-based image name parsing. So I'd rather not commit that _IP_V4_REGEXP change to master.

I think we want to keep ancestor walking to preserve the similar abd functionality.

xiekeyang commented 7 years ago

LGTM