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

LabelSelector type-checks with empty component Lists #78

Closed
ari-becker opened this issue Sep 5, 2019 · 5 comments
Closed

LabelSelector type-checks with empty component Lists #78

ari-becker opened this issue Sep 5, 2019 · 5 comments

Comments

@ari-becker
Copy link
Collaborator

I'm currently trying to fit external-dns into our Dhall-ified pipeline, and I replicated the mistake in their tutorial wherein a DeploymentSpec is created without a LabelSelector (since their Deployment doesn't have any labels in its metadata).

Currently, a DeploymentSpec bundles in its own default LabelSelector, which bundles in empty lists for matchLabels and matchExpressions. Due to --omitEmpty, this succeeds at type-checking, and the enveloping Deployment also type-checks.

However, attempting to actually apply the Deployment results in error: error validating "STDIN": error validating data: ValidationError(Deployment.spec): missing required field "selector" in io.k8s.api.apps.v1.DeploymentSpec.

Not sure what a proper solution is considering dhall-lang#691 precluded language-level support for non-empty Lists.

@f-f
Copy link
Member

f-f commented Sep 5, 2019

@ari-becker I think this is another case in which the Swagger types are "lying" (see #8), and we should integrate that type information by hand here:

-- | Some models require keys that are not in the required set,
-- but are in the docs or just work
requiredConstraints = Data.Map.fromList
[ ( ModelName "io.k8s.apimachinery.pkg.apis.meta.v1.ObjectMeta"
, Set.fromList [FieldName "name"])
]

@ari-becker
Copy link
Collaborator Author

ari-becker commented Sep 5, 2019

Reading the Kubernetes API documentation:

A label selector is a label query over a set of resources. The result of matchLabels and matchExpressions are ANDed. An empty label selector matches all objects. A null label selector matches no objects.

It sounds to me like this is really another instance where #77 comes to bear.

@arianvp
Copy link
Member

arianvp commented Sep 12, 2019

I think this is an instantiation of https://stackoverflow.com/questions/50309057/what-is-the-purpose-of-a-kubernetes-deployment-pod-selector (See first answer). It used to be the case that selector would infer a value that matches all the labels on the Deployment but that behavior was removed and now the user is expected to create a selector themselves. Because by default this would mean all labels were part of the selector, which made changing labels after the fact difficult.

About it being an instantiaton of #77 . This is a bit more funky than that. In this case it is valid for both of them to be defined or one of them to be defined. #77 is about mutually exclusive values mostly.

The anyOf operator in OpenAPI v3 would describe these semantics of it nicely. https://swagger.io/docs/specification/data-models/oneof-anyof-allof-not/#anyof

However I'm not sure how we would translate an anyOf to a dhall type. oneOf is easy as it maps nicely to a sum type, but for anyOf we need something more fancy.

@arianvp
Copy link
Member

arianvp commented Sep 12, 2019

The other ugly thing here is that matchExpression is stricly more general than matchLabels. That is, you can do everything with the matchExpresison field that you can do with the matchLabels field. Ideally, matchLables wouldn't exist at all, and would be a function of the type:

matchLabels  :: [(string, string)] ->  [LabelSelectorRequirement]

e.g.:

matchLabels:
  app: arians-cool-app
  env: production

is the same as:

matchExpressions:
  - key: app
    operator: In
    values: 
    - arians-cool-app
  - key: env
    operator: In
    values:
      - production

@Gabriella439
Copy link
Contributor

This is fixed by #110 as the selector field is now required

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

4 participants