OCM-5803 | feat: add vpc ids field to the cloud provider data struct

[oriAdler]

The new field enables filtering VPCs by VPC ids.

The equivalent change in the backend:

type CloudProviderInquiries struct {
	GCP               *GCP         `json:"gcp,omitempty"`
	Region            *CloudRegion `json:"region,omitempty"`
	KeyLocation       *string      `json:"key_location,omitempty"`
	KeyRingName       *string      `json:"key_ring_name,omitempty"`
	AWS               *AWS         `json:"aws,omitempty"`
	Version           *Version     `json:"version,omitempty"`
	AvailabilityZones []string     `json:"availability_zones,omitempty"`
	Subnets           []string     `json:"subnets,omitempty"`
-->	VpcIds            []string     `json:"vpc_ids,omitempty"`
}

[robpblake]

/lgtm

[gdbranco]

IMO, this is getting confusing. I think we should strive for a pattern. If we are going to have filters within the body of a post, can we include a filter field that maps to a specific filter for it? Like we have an AWS field which has a region ID inside it as well as we have a region outside within the overall structure

I'd like to propose to have a Filter field and we can slowly deprecate the other fields so we stop simply adding more

[robpblake]

@gdbranco Do you have a quick example of what you have in mind?

[oriAdler]

IMO, this is getting confusing. I think we should strive for a pattern. If we are going to have filters within the body of a post, can we include a filter field that maps to a specific filter for it? Like we have an AWS field which has a region ID inside it as well as we have a region outside within the overall structure

I'd like to propose to have a Filter field and we can slowly deprecate the other fields so we stop simply adding more

This is a valid proposal.
As we have already AvailabilityZones and Subnets, I rather not break the consistency and have some fields in the body and the others in the new filters field.
Maybe we can handle that as part of our tech debt, and include all the fields in the new filter struct, then we can at once communicate the changes to the clients, WDYT?

[oriAdler]

@gdbranco created this ticket https://issues.redhat.com/browse/OCM-5805, please feel free to add comments and context if needed, thanks.

[gdbranco]

Only other comment I have is that it is missing the end dot '.' to most fields in this file

[tzvatot]

IMO, this is getting confusing. I think we should strive for a pattern. If we are going to have filters within the body of a post, can we include a filter field that maps to a specific filter for it? Like we have an AWS field which has a region ID inside it as well as we have a region outside within the overall structure

I'd like to propose to have a Filter field and we can slowly deprecate the other fields so we stop simply adding more

I like the generic filter option. I do agree with @oriAdler that given the current design, we can't make breaking changes, and it will be very confusing to have subnet ids filtered in the body and the vpc-ids in the filter. Probably best to follow that new ticket as enhancement to the API (probably with a new endpoint).

[tzvatot]

LGTM