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

Kubevious removes required fields containing empty strings #20

Closed
peter-tar opened this issue May 30, 2024 · 5 comments
Closed

Kubevious removes required fields containing empty strings #20

peter-tar opened this issue May 30, 2024 · 5 comments
Assignees
Labels
bug Something isn't working enhancement New feature or request

Comments

@peter-tar
Copy link

I ran into an interesting edge case today while attempting to create a Pubsubsubscription using Config Connector.
https://cloud.google.com/config-connector/docs/reference/resource-docs/pubsub/pubsubsubscription

If one wants to set the subscription to never expire, the expirationPolicy.ttl field needs to be set to an empty string.

Specifies the "time-to-live" duration for an associated resource. The resource expires if it is not active for a period of ttl. If ttl is set to "", the associated resource never expires. A duration in seconds with up to nine fractional digits, terminated by 's'. Example - "3.5s".

Unfortunately kubevious doesn't seem to like that:

🔴 Required property "ttl" missing under "/spec/expirationPolicy"

Digging into the code a bit it seems that as a preprocessing step kubevious deletes required fields containing empty strings as a value (if there's no default value).
https://github.com/kubevious/cli/blob/fb8ab7d57f6e585e3af7b871adf8460b929d3e61/src/validation/k8s-manifest-validator.ts#L192C1-L215C6
It's a bit unfortunate because in this scenario an empty string is the expected value.

That's the extent of my understanding. I find it a bit odd from Google to use an empty string to denote "no expiration", but nothing seems to suggest that it's forbidden by the schema based on a quick search so I decided to raise it here. Let me know if there is anything else I can provide!

@rubenhak
Copy link
Collaborator

hi @peter-tar, good catch! I've experienced the same issue with GKE HealthCheckPolicy where the target ref could be a Service with an empty group field. The purpose of that code was to make mandatory fields indeed mandatory whenever they are not provided. But looks like making the linter so strict causes false-positives. I'll check it more deeper over the weekend make it more permissive.

@rubenhak
Copy link
Collaborator

@peter-tar, on a different note, since you're also using GCP Config Connector would you be willing to help with writing custom rules to validate semantics of config connector manifests? For example, it would validate that the BigQueryTable referenced by the tableRef in the PubSubSubscription is indeed present, etc...

@rubenhak rubenhak added bug Something isn't working enhancement New feature or request labels May 30, 2024
@rubenhak rubenhak self-assigned this May 30, 2024
@peter-tar
Copy link
Author

Hey @rubenhak, thanks for picking this up.

When you are talking about custom rules, do you mean these?
https://github.com/kubevious/rules-library/
To be honest, we only use the linter part currently and not the guard so I'm not super familiar with it, but I could try to help out. Validating CC semantics does sound like a very useful feature to have.

@rubenhak
Copy link
Collaborator

Hi @peter-tar, the fix is merged and released as v1.0.61.

I've added initial rules for semantic validation in GCP Config Connector SQL resources. You can try this example below. Feel free to change the instanceRef or secret references in ./demos.git/guard/60-gcp-config-connector and check the results.

git clone https://github.com/kubevious/demos.git demos.git

kubevious guard ./demos.git/crds/gcp-config-connector ./demos.git/crds/sealed-secrets ./demos.git/guard/60-gcp-config-connector

If you wish to play around with editing/writing your own rules, try this:

git clone https://github.com/kubevious/rules-library.git rules-library.git

kubevious guard --skip-community-rules ./demos.git/crds/gcp-config-connector ./demos.git/crds/sealed-secrets ./rules-library.git/gcp-config-connector ./demos.git/guard/60-gcp-config-connector

The --skip-community-rules tells the command to not to load any rules from the library. You can then pass the relevant rules from the file system: ./rules-library.git/gcp-config-connector.

Feel free to let me know if you need assistance.

@peter-tar
Copy link
Author

Hey @rubenhak
Thanks a lot! I will test out the new release.

I will also look at the semantic validation rules, thanks for the intro. I will try to get around to it in the next couple of weeks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants