Add Host normalization when building IndexConnection

[austin-denoble]

Problem

We had an issue that was logged around trying to call Client.Index() with a Host value including an http:// or https:// schema. The control plane does not attach these schemes, but when building an IndexConnection a user needs to provide a host and they could make this mistake. grpc-go is not expecting a target in NewClient that includes a scheme.

We could do a bit of normalization to handle the Host more thoroughly.

Solution

• Add normalizeHost helper function in index_connection.go. This strips out http:// & https:// prefixes, and attaches the port :443 unless the user has provided one directly. We want to leave some flexibility for making RPCs in specific scenarios.
• Add unit tests for normalizeHost.

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update
• Infrastructure change (CI configs, etc)
• Non-code change (docs, etc)
• None of the above: (explain here)

Test Plan

CI unit & integration tests should pass as normal.

To test manually, you can try and prepend schemes to the Host when calling client.Index()

indexConn, err := client.Index(pinecone.NewIndexConnParams{Host: fmt.Sprintf("https://%s", index.Host)})
if err != nil {
    panic(err)
}

stats, err := indexConn.DescribeIndexStats(context.Background())

[austin-denoble]

Moving this up to group with the other IndexConnection methods.

[haruska]

There is a bit of an oddity with stripping http:// from a host then defaulting to port 443 with TLS enabled on the connection.

[haruska]

This is a bit odd... I would expect a http:// transport to default to port 80.

However, I also don't see a user specifying http plaintext without a port for an index.

[aulorbe]

Are you sure this is meant to be deleted?

[austin-denoble]

It was in my original PR because I removed the target := fmt.Sprintf("%s:443", in.host) which was the only usage in here previously.

Once rebasing on your changes though we have those fmt.Errorf calls with the input struct checking.

[aulorbe]

What is the difference btwn this test and the 1st test? It looks like they both start and end with the same values, no?

[austin-denoble]

Ah nothing, accidentally duplicated it!

[aulorbe]

It'd be nice to add an iteration here that does https with a port, too

[austin-denoble]

Yeah, I think I meant to add that case and kept one the same through copy pasta or something. 😅

[aulorbe]

Nothing blocking, just some Qs. Great job!