Conformance: fix inappropriate test HTTP method

[grahammiln]

Change HTTP method from PUT to GET to allow a pull only registry to be tested. PUT requests should not be used during pull testing.

Signed-off-by: Graham Miln [email protected]

[sudo-bmitch]

Doesn't line 195 cause this to be skipped when doing pull only testing?

[grahammiln]

Line 195, SkipIfDisabled(pull), skips if the registry does not support pulls. So for a pull-only registry, pull is enabled and the test is run.

The affected test intends to check the error handling behaviour for a well formed but invalid request.

The test currently issues a PUT request that a pull-only registry is not required to support. Package Origin correctly responds with an unimplemented error but this too is not correctly handled by the conformance test suite.

[jdolitsky]

It is possible we imply need to expand allow HTTP status codes for this endpoint.

@grahammiln - could you tell us the HTTP status code returned for this endpoint?

It looks like 404 or 400 will be ok, and if 400 the error should be properly formatted JSON:

Expect(resp.StatusCode()).To(SatisfyAny(
	Equal(http.StatusBadRequest),
	Equal(http.StatusNotFound)))
if resp.StatusCode() == http.StatusBadRequest {
	errorResponses, err := resp.Errors()
	Expect(err).To(BeNil())

	Expect(errorResponses).ToNot(BeEmpty())
	Expect(errorCodes).To(ContainElement(errorResponses[0].Code))
}
				```

[grahammiln]

Package Origin follows the specification and returns a 405 Method Not Allowed with a JSON body error code UNSUPPORTED. Try the curl command below:

curl -v -X PUT -H "Content-Type: application/vnd.oci.image.manifest.v1+json" "https://pkg.miln.eu/v2/packageorigin/oci-distribution-conformance/manifests/sha256:totallywrong"

Returning the body:

{
  "errors": [
    {
      "code": "UNSUPPORTED",
      "message": "Registry is read-only."
    }
  ]
}

Expanding the test's accepted 400 error codes will result in testing different routes on push-pull and pull-only registries:

• On push-pull registries, a well formed but invalid request would be tested;
• On pull-only registries, an unsupported method would be tested.

This would dilute the test and is better avoided.

The proposed fix is minimal and restores the intent of the test.

I agree that a second error handling test would be useful, but that should not stop this test's bug being fixed first.

Does this help clarify the problem?

[mikebrow]

nod.. let's fix the test checking for 405 on the pull only.. and fix this push invalid test to validate we are actually on a push enabled registry..

[Jbchampions1]

Actualización