Skip to content
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.

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

Implicit ServiceSpec union? #141

Open
ari-becker opened this issue Aug 24, 2020 · 5 comments
Open

Implicit ServiceSpec union? #141

ari-becker opened this issue Aug 24, 2020 · 5 comments

Comments

@ari-becker
Copy link
Collaborator

Trying to run kubectl apply on:

apiVersion: v1
kind: Service
metadata:
  name: foo
spec:
  type: ClusterIP
  selector:
    app: foo

results in the following error: The Service "foo" is invalid: spec.ports: Required value. Putting ports: [] results in the same error.

The ports field in ServiceSpec is an Optional (List ServicePort.Type) only because of ExternalName Services (example taken from linked documentation):

apiVersion: v1
kind: Service
metadata:
  name: my-service
  namespace: prod
spec:
  type: ExternalName
  externalName: my.database.example.com

Similarly, externalName is a required field for an ExternalName ServiceSpec but should not be specified at all for other kinds of ServiceSpec.

Ran into this issue when the super-configuration that was generating a Kubernetes.Service.Type had an optional field that was being mapped to the Kubernetes Service's .spec.ports, where the field wasn't specified and the default was an empty optional.

Related: dhall-lang/dhall-lang#691

@Gabriella439
Copy link
Contributor

@ari-becker: Just to make sure I understand: do you mean that there is an API invariant that the Kubernetes server is enforcing that is not codified in the OpenAPI specification? Specifically:

  • is there an invariant that .spec.ports must be present if spec.type is not ExternalName?
  • is there an invariant that .spec.ports must be absent if spec.type is ExternalName?

@ari-becker
Copy link
Collaborator Author

@Gabriel439

is there an invariant that .spec.ports must be present if spec.type is not ExternalName?

Yes. If spec.type is undefined, the API server presumes ClusterIP by default. There are four types of Services - ClusterIP, NodePort, LoadBalancer, and ExternalName. Each of ClusterIP, NodePort, and LoadBalancer require ports to be defined, and the API server will return an error if the field is undefined or is an empty list. Only if type is defined as ExternalName does the API server accept not defining any ports.

is there an invariant that .spec.ports must be absent if spec.type is ExternalName?

I just tested this, and the API server does accept a manifest for a Service of type ExternalName which defines ports. In practice, these ports are ignored, as ExternalName is used to manually insert CNAME records into Kubernetes's cluster DNS. With that said, I can understand a user defining a port for a Service of type ExternalName as a means of documenting usage for downstream users.

@Gabriella439
Copy link
Contributor

@ari-becker: I think my inclination here is to either:

  • … ask the Kubernetes upstream to fix the OpenAPI spec to codify these invariants, or:
  • … created a more strongly-typed representation downstream of dhall-kubernetes (e.g. in dhall-packages or whatever evolves into the "Helm for Dhall").

The main reason why is that we're slowly trying to generalize the dhall-kubernetes logic to dhall-openapi, so the less we deviate from the OpenAPI specification the fewer Kubernetes-specific adjustments the code will have to make.

I do agree, though, that this sounds like the obvious more strongly-typed representation would be a Dhall union with at least two alternatives (one alternative for ExternalName and at least one alternative for other types)

@ari-becker
Copy link
Collaborator Author

@Gabriel439 Asking Kubernetes upstream to fix the OpenAPI spec is non-trivial.

The spec itself is a computer-generated ~ 5 MB document, which seems to be generated from the JSON tags on the Golang structs of the API objects, see e.g. ServiceSpec (at least that's my best guess without spending much more time in the codebase). For better or worse, these are long-stabilized APIs whose validations are unlikely to be changed at this point.

I'm wondering about a different approach - keep moving towards a generalized dhall-openapi, but trying to find a way to "append" additional type-level assertions after the initial generation to form the final types. I'm not sure how that would really look though.

@arianvp
Copy link
Member

arianvp commented Oct 3, 2020

not providing ports is also supported for headless services by the way, not just ExternalNames

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants