Skip to content

Check API for optional vs required for next revision #10915

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
sivchari opened this issue Jul 20, 2024 · 6 comments
Closed

Check API for optional vs required for next revision #10915

sivchari opened this issue Jul 20, 2024 · 6 comments
Labels
kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete.

Comments

@sivchari
Copy link
Member

sivchari commented Jul 20, 2024 •
edited
Loading

This issue is similar with #6416
I listed the fields that we should discuss about which the data type is better for each field for next revision.

I listed the field that is optional, but the type isn't pointer or that isn't optional, but the type is pointer.
ref: https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#optional-vs-required

They have the +optional comment tag in Go.
They are a pointer type in the Go definition (e.g. AwesomeFlag *SomeFlag) or have a built-in nil value (e.g. maps and slices).
The API server should allow POSTing and PUTing a resource with this field unset.

Sorry if this is too delicated for the community, but I think it should do it to be good stable API because.

Thanks.

ClusterSpec

  • .ClusterNetwork.ServiceDomain
  • .Topology.Controlplane
  • .Topology.Controlplane.Metadata
  • .Topology.Workers.MachineDeployments.Metadata
  • .Topology.Workers.MachinePools.Metadata
  • .Topology.Variables.DefinitionFrom

ClusterStatus

  • .FailureDomains
  • .FailureDomains.ControlPlane
  • .FailureDomains.Attributes
  • .Phase
  • .InfrastructureReady
  • .ControlPlaneReady
  • .Conditions
  • .ObservedGeneration

ClusterClassSpec

  • .Infrastructure
  • .ControlPlane
  • .ControlPlane.Metadata
  • .Workers
  • .Workers.MachineDeployments.MinReadySeconds
  • .Workers.MachineDeployments.Strategy
  • .Workers.MachineDeployments.Template.Metadata
  • .Workers.MachinePools.MinReadySeconds
  • .Workers.MachinePools.Template.Metadata
  • .Variables..Metadata
  • .Patches.Example
  • .Patches.UniqueItems
  • .Patches.Format
  • .Patches.Pattern
  • .Patches.ExclusiveMaximum
  • .Patches.ExclusiveMinimum
  • .Patches.XPreserveUnknownFields
  • .Patches.XValidations.Message
  • .Patches.XValidations.MessageExpression
  • .Patches.XValidations.Reason
  • .Patches.XValidations.FieldPath
  • .PatchesDefinitions.Selector.MatchResources.ControlPlane
  • .PatchesDefinitions.Selector.MatchResources.InfrastructureCluster

ClusterClassStatus

  • .Conditions
  • ObservedGeneration
  • .Variables.DefinitionsConflict
  • .Variables.Definitions.Metadata

MachineStatus

  • .Addresses
  • .Phase
  • .BootstrapReady
  • .InfrastructureReady
  • .ObservedGeneration
  • .Conditions

MachineDeploymentSpec

  • .Paused
  • .Strategy.Type

MachineDeploymentStatus

  • .ObservedGeneration
  • .Selector
  • .Replicas
  • .UpdatedReplicas
  • .ReadyReplicas
  • .AvailableReplicas
  • .UnavailableReplicas
  • .Phase
  • .Conditions

MachineHealthCheckStatus

  • .ExpectedMachines
  • .CurrentHealthy
  • .RemediationsAllowed
  • .ObservedGeneration
  • .Conditions

MachineSetSpec

  • .MinReadySeconds
  • .DeletePolicy
  • .Template
  • .Template.ObjectMeta
  • .Template.Spec

MachineSetStatus

  • .Selector
  • .Replicas
  • .FullyLabeledReplicas
  • .ReadyReplicas
  • .AvailableReplicas
  • .ObservedGeneration
  • .Conditions

KubeadmConfigSpec

  • .ClusterConfiguration.Etcd
  • .ClusterConfiguration.Etcd.Local.DataDir
  • .ClusterConfiguration.Etcd.Local.ExtraArgs
  • .ClusterConfiguration.Networking
  • .ClusterConfiguration.Networking.ServiceSubnet
  • .ClusterConfiguration.Networking.PodSubnet
  • ..ClusterConfiguration.Networking.DNSDomain
  • ..ClusterConfiguration.KubernetesVersion
  • .ClusterConfiguration.ControlPlaneEndpoint
  • .ClusterConfiguration.APIServer
  • .ClusterConfiguration.APIServer.ControlPlaneComponent.ExtraVolumes.ReadyOnly
  • .ClusterConfiguration.APIServer.ControlPlaneComponent.ExtraVolumes.PathType
  • .ClusterConfiguration.ControllerManager
  • .ClusterConfiguration.Scheduler
  • .ClusterConfiguration.DNS
  • .ClusterConfiguration.CertificatesDir
  • .ClusterConfiguration.ImageRepository
  • .ClusterConfiguration.ClusterName
  • .InitConfiguration.BootstrapTokens.Token
  • .InitConfiguration.BootstrapTokens.Description
  • .InitConfiguration.NodeRegistration
  • .InitConfiguration.NodeRegistration.Name
  • .InitConfiguration.NodeRegistration.CRISocket
  • .InitConfiguration.NodeRegistration.ImagePullPolicy
  • .InitConfiguration.LocalAPIEndpoint.AdvertiseAddress
  • .InitConfiguration.LocalAPIEndpoint.BindPort
  • .InitConfiguration.Patches.Directory
  • .JoinConfiguration.NodeRegistration
  • .JoinConfiguration.CACertPath
  • .JoinConfiguration.Discovery.BootstrapToken.APIServerEndpoint
  • .JoinConfiguration.Discovery.BootstrapToken.UnsageSkipCAVerification
  • .JoinConfiguration.Discovery.File.KubeConfig.Server
  • .JoinConfiguration.Discovery.File.KubeConfig.TLSServerName
  • .JoinConfiguration.Discovery.File.KubeConfig.InsecureSkipTLSVerify
  • .JoinConfiguration.Discovery.File.KubeConfig.ProxyURL
  • .JoinConfiguration.Discovery.TLSBootstrapToken
  • .JoinConfiguration.ControlPlane.LocalEndpoint
  • JoinConfiguration.SkipPhases
  • .Files.Owner
  • .Files.Permissions
  • .Files.Encoding
  • .Files.Append
  • .Fiels.Content
  • .Format
  • .Ignition.ContainerLinuxConfig.AdditionalConfig
  • .Ignition.ContainerLinuxConfig.Strict

KubeadmConfigStatus

  • .Ready
  • .DataSecretName
  • .FailureReason
  • .FailureMessage
  • .ObservedGeneration
  • .Conditions

KubeadmControlPlaneSpec

  • .MachineTemplate.ObjectMeta
  • .RolloutStrategy.Type
  • .RemediationStrategy.RetryPeriod

KubeadmControlPlaneStatus

  • .Selector
  • .Replicas
  • .UpdatedReplicas
  • .ReadyReplicas
  • .UnavailableReplicas
  • .Initialized
  • .Body
  • .FailureReason
  • .ObservedGeneration
  • .Conditions

KubeadmControlPlaneTemplateSpec

  • .Template.ObjectMeta
  • .Template.Spec.MachineTemplate.ObjectMeta
@k8s-ci-robot k8s-ci-robot added needs-priority Indicates an issue lacks a `priority/foo` label and requires one. needs-kind Indicates a PR lacks a `kind/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jul 20, 2024
@k8s-ci-robot
Copy link
Contributor

This issue is currently awaiting triage.

If CAPI contributors determine this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@fabriziopandini fabriziopandini changed the title Improve API definition for next revision Check API for optional vs required for next revision Jul 31, 2024
@fabriziopandini
Copy link
Member

We have to take a closer look
Some time ago we even considered to implement a linter to enforce some rules about optional vs required.
But first we need to figure out if the rules defined in API conventions apply to CRDs / To CAPI (we found some nuances in the past)

@fabriziopandini fabriziopandini added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API labels Jul 31, 2024
@k8s-ci-robot k8s-ci-robot removed needs-kind Indicates a PR lacks a `kind/foo` label and requires one. labels Jul 31, 2024
@fabriziopandini fabriziopandini added the priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. label Jul 31, 2024
@k8s-ci-robot k8s-ci-robot removed the needs-priority Indicates an issue lacks a `priority/foo` label and requires one. label Jul 31, 2024
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 29, 2024
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle rotten
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Nov 28, 2024
@chrischdi
Copy link
Member

/close

We are leaning towards using a linter for applying standard rules consistently (see e.g. #11733).

@k8s-ci-robot
Copy link
Contributor

@chrischdi: Closing this issue.

In response to this:

/close

We are leaning towards using a linter for applying standard rules consistently (see e.g. #11733).

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@chrischdi @fabriziopandini @k8s-ci-robot @k8s-triage-robot @sivchari