opened
praneetloke
username_0
Also:
- removed the Token property from RenewUpdateLeaseRequest
- bumped the API version number to 5
Change RenewUpdateLease to use the update access token as the token in the Authorization header
issue
created
praneetloke
username_0
When this is ready for review (waiting on a service change), I'll add this change to the appropriate changelog entry at that time.
comment
created
praneetloke
username_0
Would removing the property only affect the newly released CLI and not the old/current versions? When users update to the new CLI version, the API version would be 5 and the CLI would no longer send the access token _and_ the update token for the `renew_lease` call, instead sending just the update token in the `Authorization` header. That's what the service would expect for API version 5+.
comment
created
chrsmith
username_1
@username_0 , you're right about how the code will impact newer builds of the Pulumi CLI, etc.

Why I'm suggesting you back out the changes in `apitype/update.go` is because the Pulumi Service's codebase still has a dependency on the `pulumi/pulumi` codebase, when it validates requests from older CLIs.

If we were to remove `RenewUpdateLeaseRequest.Token`, then the next time we update the `pulumi-service` repo to depending on a newer version of the `pulumi/pulumi` code, it would break the build. Since `pulumi-service` (I assume) still needs to "see" and reference the `RenewUpdateLeaseRequest::Token` field when it is processing requests from older CLI clients.

So in a vacuum, it makes sense to remove it in this repo. But because of that dependency and backwards-compatibility for the Pulumi Service, we might want to keep it around.

Does that make sense? Or am I missing something?

Keeping that vestigial field around in `pulumi/pulumi` isn't the only solution though. We could make a copy of the type (specifically it's JSON definition), and put that into the `pulumi-service` repo. So that `pulumi/pulumi` is always "current" and doesn't have vestigial code around for older, unused API shapes.
comment
created
praneetloke
username_0
This seems like a good idea, but I am not sure which is better: 1.) having an unused field in this repo, so that a downstream repo can use it or, 2.) declare a "compat" type in the downstream repo for backward compat reasons. I do think 1.) is better for the purposes of knowing what the CLI to the service, so that it is not hidden, at least until the minimum CLI version is raised and it is no longer a problem.
comment