Openapi TestConnection new method

[mikeletux]

This PR does not have any issue associated.

Description

When creating a catalog and subscribing to an external one, VCD uses the endpoint /cloudapi/1.0.0/testConnection/ to check before hand if the endpoint where the user is trying to subscribe the external catalog from exists.

Detailed description

In order to implement the external catalog subscription in the Terraform provider for VCD when creating a catalog, TestConnection functionality needs to be implemented since before subscribing, VCD does this previous sanity check.

This has been implemented as a Client method, since it doesn't belong to any VCD resource and it's a client helping tool.

Also, I propose a slight change to Client.OpenApiPostItem since to use TestConnection, I had to do a POST request against /cloudapi/1.0.0/testConnection/. This results in a 200OK, instead of 201 or 202.
Client.OpenApiPostItem wasn't covering this case, so the function worked but it wasn't unmarshalling the response into the needed struct.
This last point is open to discussion, as it might be a better way to handle this.

🚀

[lvirbalas]

In order to implement the external catalog subscription in the Terraform provider for VCD when creating a catalog, TestConnection functionality needs to be implemented since before subscribing, VCD does this previous sanity check.

I guess I'm still missing the use case: who/where/when will call the new OpenApiTestConnection function?

Thanks!

[mikeletux]

In order to implement the external catalog subscription in the Terraform provider for VCD when creating a catalog, TestConnection functionality needs to be implemented since before subscribing, VCD does this previous sanity check.

I guess I'm still missing the use case: who/where/when will call the new OpenApiTestConnection function?

Thanks!

Yeah, maybe I didn't get much context.
When creating a catalog using the VCD UI (and as long as you have permission to subscribe to an external catalog), you can create a catalog subscribing from an external one. Please check the first figure:

If you check the HTTP traffic using a traffic sniffer, when you fill everything and click the OK button, you can see two POST calls being generated by the frontend.

• The first one is to TestConnection. That endpoint checks that the remote endpoint where you are trying to subscribe the external catalog from, exists.
• The second one is the actual call that creates the catalog for that Org.

Let me know if this make it clear 😄

[lvirbalas]

LGTM, and thanks for explaining how we will use it at #447 (comment)

[Didainius]

Two parts

OpenAPI function adjustment

Looks good in general, but because functions in openapi.go are a backbone for all OpenAPI requests I have a few minor addition requests which I left inline, but also here as I can't comment untouched code lines.

• Could you also adjust sister function OpenApiPostItemSync so that these behave the same way in sync operations. Also it's documentation. For a test you could manually submit the payload for testConnnection to see if it actually handles it well.
• A changelog entry needs a separate line about adjustment of these functions and the reason for it.
• I already mentioned in explicit content but because this is no longer a single status code that guides the operation of these functions it would be beneficial to explicitly log status code as we pick the path.
  ** https://github.com/vmware/go-vcloud-director/pull/447/files#diff-585772f0a826713495acbb0951db5ff9d9f252426fca85e2e112581e5d90a29dR295
  ** https://github.com/vmware/go-vcloud-director/pull/447/files#diff-585772f0a826713495acbb0951db5ff9d9f252426fca85e2e112581e5d90a29dR191-R193 (here once you adjust it to handle both codes)

OpenApiTestConnection

The test Test_OpenApiTestConnection has issue on 10.2 versions:

• 10.2.0 - FAILS:

go test  -count 1 -tags ALL -check.vv -check.f OpenApiTestConnection -timeout=155m .
START: api_vcd_test.go:477: TestVCD.SetUpSuite
Running on VCD https://HOST/api (version 10.2.0.17008054 built at 2020-10-08 20:53:08 +0000 UTC)
as user administrator@System (using password)
Skipping all vapp tests because one of the following wasn't given: Network, StorageProfile, Catalog, Catalogitem
PASS: api_vcd_test.go:477: TestVCD.SetUpSuite   19.153s

START: openapi_test.go:265: TestVCD.Test_OpenApiTestConnection
openapi_test.go:304:
    check.Assert(err, IsNil)
... value *errors.errorString = &errors.errorString{s:"error creating Edge Gateway: error in HTTP POST request: VCD_50006 - [ 71b4c62b-90f2-4b16-9668-448b7d3358e5 ] Json processing error.\n - Unrecognized field \"hostnameVerificationAlgorithm\" (class com.vmware.vcloud.rest.openapi.model.Connection), not marked as ignorable (5 known properties: \"host\", \"port\", \"proxyConnection\", \"secure\", \"timeout\"])\n at [Source: (org.apache.cxf.transport.http.AbstractHTTPDestination$1); line: 6, column: 37] (through reference chain: com.vmware.vcloud.rest.openapi.model.Connection[\"hostnameVerificationAlgorithm\"])"} ("error creating Edge Gateway: error in HTTP POST request: VCD_50006 - [ 71b4c62b-90f2-4b16-9668-448b7d3358e5 ] Json processing error.\n - Unrecognized field \"hostnameVerificationAlgorithm\" (class com.vmware.vcloud.rest.openapi.model.Connection), not marked as ignorable (5 known properties: \"host\", \"port\", \"proxyConnection\", \"secure\", \"timeout\"])\n at [Source: (org.apache.cxf.transport.http.AbstractHTTPDestination$1); line: 6, column: 37] (through reference chain: com.vmware.vcloud.rest.openapi.model.Connection[\"hostnameVerificationAlgorithm\"])")

FAIL: openapi_test.go:265: TestVCD.Test_OpenApiTestConnection

START: api_vcd_test.go:1549: TestVCD.TearDownSuite
PASS: api_vcd_test.go:1549: TestVCD.TearDownSuite       0.002s

OOPS: 0 passed, 1 FAILED

• 10.2.2 - FAILS

go test  -count 1 -tags ALL -check.vv -check.f OpenApiTestConnection -timeout=155m .
START: api_vcd_test.go:477: TestVCD.SetUpSuite
Running on VCD https://HOST/api (version 10.2.2.18634228 built at 2021-09-15 17:30:35 +0000 UTC)
as user administrator@System (using password)
Skipping all vapp tests because one of the following wasn't given: Network, StorageProfile, Catalog, Catalogitem
PASS: api_vcd_test.go:477: TestVCD.SetUpSuite   21.967s

START: openapi_test.go:265: TestVCD.Test_OpenApiTestConnection
openapi_test.go:304:
    check.Assert(err, IsNil)
... value *errors.errorString = &errors.errorString{s:"error creating Edge Gateway: error in HTTP POST request: VCD_50006 - [ e08d6df1-4eb0-42ec-b090-cda5ef4dedeb ] Json processing error.\n - Unrecognized field \"hostnameVerificationAlgorithm\" (class com.vmware.vcloud.rest.openapi.model.Connection), not marked as ignorable (5 known properties: \"host\", \"port\", \"proxyConnection\", \"secure\", \"timeout\"])\n at [Source: (org.apache.cxf.metrics.interceptors.CountingInputStream); line: 6, column: 37] (through reference chain: com.vmware.vcloud.rest.openapi.model.Connection[\"hostnameVerificationAlgorithm\"])"} ("error creating Edge Gateway: error in HTTP POST request: VCD_50006 - [ e08d6df1-4eb0-42ec-b090-cda5ef4dedeb ] Json processing error.\n - Unrecognized field \"hostnameVerificationAlgorithm\" (class com.vmware.vcloud.rest.openapi.model.Connection), not marked as ignorable (5 known properties: \"host\", \"port\", \"proxyConnection\", \"secure\", \"timeout\"])\n at [Source: (org.apache.cxf.metrics.interceptors.CountingInputStream); line: 6, column: 37] (through reference chain: com.vmware.vcloud.rest.openapi.model.Connection[\"hostnameVerificationAlgorithm\"])")

FAIL: openapi_test.go:265: TestVCD.Test_OpenApiTestConnection

START: api_vcd_test.go:1549: TestVCD.TearDownSuite
PASS: api_vcd_test.go:1549: TestVCD.TearDownSuite       0.001s

OOPS: 0 passed, 1 FAILED
--- FAIL: Test (22.90s)

• 10.3.2 - OK

[mikeletux]

@Didainius some comments regarding the changes that I did.

• Apart from the changes you suggested to Client.OpenApiPostItemSync now the method is returning an error if the status code that is returned is not 200 or 201. This wasn't the previous behavior. I did it because the function description actually says that if return code is different from 200 or 201 it errors, and also because when writing the test if wasn't impossible to find out if POST against an endpoint that returns 200OK was handled appropriately.
• Add a workaround to Client.TestConnection so if API max version is below 36.0, sets some fields not supported to empty so they are not send in the POST request and the function doesn't error (I tested this on 10.2.2. Could you please test it on 10.2.0?)
• Add a test in Test_OpenApiInlineStructCRUDRoles that manually builds TestConnection payload and send it using Client.OpenApiPostItemSync to check that this function handles correctly 200OK.

Let me know if you're concerned about anything else! 🚀

[vbauzys]

I would say we need wrapper function too, which would have only minimum values for input like URL and everything else default. I guess in UI you provide URL.

[mikeletux]

I would say we need wrapper function too, which would have only minimum values for input like URL and everything else default. I guess in UI you provide URL.

This is interesting. Let's see what @lvirbalas @Didainius @adambarreiro have to say about it.
I could grab the default values that the UI uses and write a wrapper.

[Didainius]

This is interesting. Let's see what @lvirbalas @Didainius @adambarreiro have to say about it.
I could grab the default values that the UI uses and write a wrapper.

Go ahead if this is easy to make it (no advanced computations for fields)

[vbauzys]

I would say we need wrapper function too, which would have only minimum values for input like URL and everything else default. I guess in UI you provide URL.

This is interesting. Let's see what @lvirbalas @Didainius @adambarreiro have to say about it. I could grab the default values that the UI uses and write a wrapper.

how the current function is used in terraform? how does call look like?

[mikeletux]

how the current function is used in terraform? how does call look like?

Is not yet being used, but I'll use it when enhancing the Catalog resource, when it comes to create a new catalog subscribing from an external one.
Before doing anything, check that the VCD instance where the catalog you're going to subscribe from, exist.

[vbauzys]

how the current function is used in terraform? how does call look like?

Is not yet being used, but I'll use it when enhancing the Catalog resource, when it comes to create a new catalog subscribing from an external one. Before doing anything, check that the VCD instance where the catalog you're going to subscribe from, exist.

So then you see how kind wrapper is better :)

[mikeletux]

@vbauzysvmware added the method TestConnectionWithDefaults so it gets a subscription URL and returns if it was possible to connect.

[Didainius]

Thanks for fixes. I am approving - just fix the typo in a separate comment.

[Didainius]

This remaining thing to fix.

[mikeletux]

Rephrased that so it makes sense now. Thanks for spotting that 🚀

[vbauzys]

Thank you

[Didainius]

Closing in favor of #501