Skip to content

fix: PC-12544 Add validation for components list to be required (even if empty) #403

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

Merged
merged 1 commit into from
May 9, 2024

Conversation

marcinlawnik
Copy link
Contributor

Motivation

New SLO has elements at depth of 4:

objectives:
  composite:
    components:
      objectives:

A list of objectives can be empty.

It may be possible that a typo is made at the highest depth (objectives), which results in no error, but the list being empty (while the user expects the objectives to be there).

Summary

Made the components list required, so that an empty array(YAML) / slice(Go) needs to be passed instead of omitted YAML property or nil slice.

Related changes

None

Testing

Added unit tests, sample YAML with the issue:

- apiVersion: n9/v1alpha
  kind: SLO
  metadata:
    name: slo-test
    project: sample-project
  spec:
    alertPolicies: []
    budgetingMethod: Occurrences
    objectives:
    - displayName: Test
      name: objective-1
      composite:
        maxDelay: 20m
        objectives: # missing components parent`
          - project: splunk-direct
            slo: splunk-direct-counts-calendar
            objective: objective-1
            weight: 1
            whenDelayed: CountAsGood
          - project: splunk-direct
            slo: splunk-direct-raw-calendar
            objective: objective-3
            weight: 1
            whenDelayed: CountAsGood
          - project: thousandeyes
            slo: net-loss-rolling-timeslices
            objective: objective-1
            weight: 1
            whenDelayed: CountAsGood
      target: 0.9
    service: service
    timeWindows:
    - count: 1
      isRolling: true
      unit: Day
  status:
    updatedAt: "2022-01-21T11:22:14Z"

Correct YAML:

- apiVersion: n9/v1alpha
  kind: SLO
  metadata:
    name: slo-test
    project: sample-project
  spec:
    alertPolicies: []
    budgetingMethod: Occurrences
    objectives:
    - displayName: Test
      name: objective-1
      composite:
        maxDelay: 20m
        components:
          objectives:
            - project: splunk-direct
              slo: splunk-direct-counts-calendar
              objective: objective-1
              weight: 1
              whenDelayed: CountAsGood
            - project: splunk-direct
              slo: splunk-direct-raw-calendar
              objective: objective-3
              weight: 1
              whenDelayed: CountAsGood
            - project: thousandeyes
              slo: net-loss-rolling-timeslices
              objective: objective-1
              weight: 1
              whenDelayed: CountAsGood
      target: 0.9
    service: service
    timeWindows:
    - count: 1
      isRolling: true
      unit: Day
  status:
    updatedAt: "2022-01-21T11:22:14Z"

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@marcinlawnik marcinlawnik merged commit db92842 into main May 9, 2024
5 checks passed
@marcinlawnik marcinlawnik deleted the PC-12544-components-validation-bug branch May 9, 2024 11:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working go patch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants