Alpha field in the stable API should follow best practices

[pooneh-m]

An Alpha field is added but not following best practices of introducing an unstable version to a stable API:

• The field should be a pointer
• Use // +optional tag
• omitempty for GameServerStatus.Alpha
• Protected by feature gate for the API, storage and validation.

[markmandel]

Thanks for sharing this 👍 ! I went hunting for some kind of best practice here, and couldn't find one!
(Curious - where did you find this? This is a very strange url)

The field should be a pointer

Sounds like a good idea. Let's do this.

// Use // +optional tag
Sounds good! Wondering what the change there will be to the generated client, but we will find out!

emitempty for GameServerStatus.Alpha

I assume you mean omitempty ? We have this, but since it's not a pointer, it's probably not that useful.

Protected by feature gate for the API, storage and validation.

Maybe we clear the Alpha state on "ApplyDefaults" if one exists? Since we can't directly control the storage since we are using CRDs and not a full API Extension.

I feel like creating new mutating webhooks to remove this value on update/create if they don't exist already might not be worth the effort.

How does that sound?

[pooneh-m]

Thanks for sharing this ! I went hunting for some kind of best practice here, and couldn't find one!
(Curious - where did you find this? This is a very strange url)

I updated the URL to its origin.

The field should be a pointer

Sounds like a good idea. Let's do this.

// Use // +optional tag
Sounds good! Wondering what the change there will be to the generated client, but we will find out!

emitempty for GameServerStatus.Alpha

I assume you mean omitempty ? We have this, but since it's not a pointer, it's probably not that useful.

We have it on GameServer.Alpha but not GameServerStatus.Alpha
Thanks, yes I mean omitempty. :)

Protected by feature gate for the API, storage and validation.

Maybe we clear the Alpha state on "ApplyDefaults" if one exists? Since we can't directly control the storage since we are using CRDs and not a full API Extension.

ApplyDefaults is a good idea and easy to implement.

I feel like creating new mutating webhooks to remove this value on update/create if they don't exist already might not be worth the effort.

How does that sound?

👍

[markmandel]

We have it on GameServer.Alpha but not GameServerStatus.Alpha

Ah! Excellent catch! Good point.

Thanks for finding this - I was wondering how other groups solved this problem - this is great to know there is prior art we can leverage 🙌