PCCM-28439 Metal Client changes to support PCE HA Zones

[patricia-dibenedetto]

Following updates are made to Swagger definition:

projects-info will be updated to allow multiple 'siteID' query parameters to be specified.
Update Project allows permitted sites input.

2 updated files for review are:
v1/api/swagger/components/parameters/SiteID.yaml
v1/api/swagger/components/schemas/UpdateProject.yaml

All other files are auto generated.

[dbozzato81]

Changes looks good but I'm wondering if the siteID change from a string to an array constitute a breaking change for consumers of the API.

[patricia-dibenedetto]

Changes looks good but I'm wondering if the siteID change from a string to an array constitute a breaking change for consumers of the API.

can you be more explicit or give example of your concern? this the openapi spec change, the qcli and Metal terraform provider will need to update this dependency along with any other changes to handle the change.

[dbozzato81]

Changes looks good but I'm wondering if the siteID change from a string to an array constitute a breaking change for consumers of the API.

can you be more explicit or give example of your concern? this the openapi spec change, the qcli and Metal terraform provider will need to update this dependency along with any other changes to handle the change.

I'm wondering if there are consumers (teams) of the generated go client itself, I'm not thinking about qCLI or Terraform. If that's not the case nothing to worry about.

[mchuang3]

I'd recommend being backwards compatible as much as possible. If you define a new "SiteIDs" or "SiteIDList" that's array of string, and leave existing "SiteID" as is, then there's less chances of causing issues. API handler code would know how to deal with either query parameters. This will allow consumers of Project API (i.e. CM-VMaaS, PCO, etc.) to migrate at their own leisure without breaking current code.
Clarification - ProjectsInfo API will accept either SiteID or SiteIDList as query params, and handle appropriately.

[patricia-dibenedetto]

I'd recommend being backwards compatible as much as possible. If you define a new "SiteIDs" or "SiteIDList" that's array of string, and leave existing "SiteID" as is, then there's less chances of causing issues. API handler code would know how to deal with either query parameters. This will allow consumers of Project API (i.e. CM-VMaaS, PCO, etc.) to migrate at their own leisure without breaking current code. Clarification - ProjectsInfo API will accept either SiteID or SiteIDList as query params, and handle appropriately.

Metal backend code was changed to accept array of string while back. From URL perspective, if user enters just 1 site it is handled correctly, so that it's backwards compatible. This PR is about the OpenAPI and changes will be made to qcli/TF provider that consumes this dependency which is what consumers of this API use besides straight HTTP URL, did not know we are to support other use cases

[mchuang3]

Metal backend code was changed to accept array of string while back. From URL perspective, if user enters just 1 site it is handled correctly, so that it's backwards compatible. This PR is about the OpenAPI and changes will be made to qcli/TF provider that consumes this dependency which is what consumers of this API use besides straight HTTP URL, did not know we are to support other use cases

Sorry, I'm not following whether there's a breaking change or not. Can you give examples of URL generated before and after this change? Putting it another way, if a user is using old API (whether their own code or using old qcli/TF versions) to talk to a Metal backend that has new code changes already, will it work or not? If so, can you point me to the backend code that's handling either string or array of strings simultaneously? Thanks.

[patricia-dibenedetto]

Metal backend code was changed to accept array of string while back. From URL perspective, if user enters just 1 site it is handled correctly, so that it's backwards compatible. This PR is about the OpenAPI and changes will be made to qcli/TF provider that consumes this dependency which is what consumers of this API use besides straight HTTP URL, did not know we are to support other use cases

Sorry, I'm not following whether there's a breaking change or not. Can you give examples of URL generated before and after this change? Putting it another way, if a user is using old API (whether their own code or using old qcli/TF versions) to talk to a Metal backend that has new code changes already, will it work or not? If so, can you point me to the backend code that's handling either string or array of strings simultaneously? Thanks.

GET /projects-info?siteid=site1 (current usage)
GET /projects-info?siteid=site1&siteid=site2 (new usage allows multiple values of same parameter, also single value)

backend code at quake/portal/pkg/project/info.go

[anurag-shrivastav]

I don't think the changes will break existing usage, but maybe TF is affected (as Denis pointed out)? Will leave to Mike/Denis to approve.

[patricia-dibenedetto]

I don't think the changes will break existing usage, but maybe TF is affected (as Denis pointed out)? Will leave to Mike/Denis to approve.

TF Update Project is affected to handle optional input for permitted sites (not a breaking change).
Other change regarding projects-info does not apply to TF (not exposed).

[mchuang3]

GET /projects-info?siteid=site1 (current usage)
GET /projects-info?siteid=site1&siteid=site2 (new usage allows multiple values of same parameter, also single value)

backend code at quake/portal/pkg/project/info.go

Ok, backend code that I was interested in is the Project API server here, not the business logic info.go. I see that this should be no issue w/ compatibility:

1. Old client, new backend - works since it is treated as query for a single site.
2. new client, old backend - works since old code uses first siteID found.

But I had concerns about the new client & the format it is putting out the multiple siteIDs. Looked like it was using a CSV format (site=site1,site2,site3), rather than the multiple siteid queries. Looks like you fixed it with the "explode" option. So I think my concerns are addressed now. Thanks.
Deferring to @dbozzato81 to approve if he has no other concerns.