Skip to content

Commit 1140fe0

Browse files
committed
Document a Kubernetes bug with the duration format
Fractional numbers are valid but not parsed correctly. See: https://www.github.com/kubernetes/kube-openapi/issues/523
1 parent 8dbe427 commit 1140fe0

File tree

7 files changed

+29
-6
lines changed

7 files changed

+29
-6
lines changed

config/crd/bases/postgres-operator.crunchydata.com_pgadmins.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1952,7 +1952,7 @@ spec:
19521952
retentionPeriod:
19531953
description: |-
19541954
How long to retain log files locally. An RFC 3339 duration or a number
1955-
and unit: `3d`, `4 weeks`, `12 hr`, etc.
1955+
and unit: `12 hr`, `3d`, `4 weeks`, etc.
19561956
format: duration
19571957
maxLength: 20
19581958
minLength: 1

config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11485,7 +11485,7 @@ spec:
1148511485
retentionPeriod:
1148611486
description: |-
1148711487
How long to retain log files locally. An RFC 3339 duration or a number
11488-
and unit: `3d`, `4 weeks`, `12 hr`, etc.
11488+
and unit: `12 hr`, `3d`, `4 weeks`, etc.
1148911489
format: duration
1149011490
maxLength: 20
1149111491
minLength: 1

internal/testing/cmp/cmp.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
package cmp
66

77
import (
8+
"regexp"
89
"strings"
910

1011
gocmp "github.com/google/go-cmp/cmp"
@@ -46,10 +47,15 @@ func Contains(collection, item any) Comparison {
4647
// succeeds if the values are equal. The comparison can be customized using
4748
// comparison Options. See [github.com/google/go-cmp/cmp.Option] constructors
4849
// and [github.com/google/go-cmp/cmp/cmpopts].
49-
func DeepEqual(x, y any, opts ...gocmp.Option) Comparison {
50+
func DeepEqual[T any](x, y T, opts ...gocmp.Option) Comparison {
5051
return gotest.DeepEqual(x, y, opts...)
5152
}
5253

54+
// Len succeeds if actual has the expected length.
55+
func Len[Slice ~[]E, E any](actual Slice, expected int) Comparison {
56+
return gotest.Len(actual, expected)
57+
}
58+
5359
// MarshalContains converts actual to YAML and succeeds if expected is in the result.
5460
func MarshalContains(actual any, expected string) Comparison {
5561
b, err := yaml.Marshal(actual)
@@ -71,6 +77,6 @@ func MarshalMatches(actual any, expected string) Comparison {
7177
// Regexp succeeds if value contains any match of the regular expression re.
7278
// The regular expression may be a *regexp.Regexp or a string that is a valid
7379
// regexp pattern.
74-
func Regexp(re any, value string) Comparison {
80+
func Regexp[RE *regexp.Regexp | ~string](re RE, value string) Comparison {
7581
return gotest.Regexp(re, value)
7682
}

internal/testing/validation/pgadmin_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"sigs.k8s.io/yaml"
1616

1717
"github.com/crunchydata/postgres-operator/internal/controller/runtime"
18+
"github.com/crunchydata/postgres-operator/internal/testing/cmp"
1819
"github.com/crunchydata/postgres-operator/internal/testing/require"
1920
"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
2021
)
@@ -49,7 +50,7 @@ func TestPGAdminInstrumentation(t *testing.T) {
4950
//nolint:errorlint // This is a test, and a panic is unlikely.
5051
status := err.(apierrors.APIStatus).Status()
5152
assert.Assert(t, status.Details != nil)
52-
assert.Equal(t, len(status.Details.Causes), 2)
53+
assert.Assert(t, cmp.Len(status.Details.Causes, 2))
5354

5455
for _, cause := range status.Details.Causes {
5556
assert.Equal(t, cause.Field, "spec.instrumentation.logs.retentionPeriod")

pkg/apis/postgres-operator.crunchydata.com/v1beta1/instrumentation_types.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,11 @@ type InstrumentationLogsSpec struct {
5454
Exporters []string `json:"exporters,omitempty"`
5555

5656
// How long to retain log files locally. An RFC 3339 duration or a number
57-
// and unit: `3d`, `4 weeks`, `12 hr`, etc.
57+
// and unit: `12 hr`, `3d`, `4 weeks`, etc.
5858
// ---
5959
// Kubernetes ensures the value is in the "duration" format, but go ahead
6060
// and loosely validate the format to show some acceptable units.
61+
// NOTE: This rejects fractional numbers: https://github.com/kubernetes/kube-openapi/issues/523
6162
// +kubebuilder:validation:Pattern=`^(PT)?( *[0-9]+ *(?i:(h|hr|d|w|wk)|(hour|day|week)s?))+$`
6263
//
6364
// `controller-gen` needs to know "Type=string" to allow a "Pattern".

pkg/apis/postgres-operator.crunchydata.com/v1beta1/shared_types.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,9 @@ import (
4040
// - https://docs.k8s.io/reference/using-api/cel/#type-system-integration
4141
// - https://github.com/google/cel-spec/blob/-/doc/langdef.md#types-and-conversions
4242
//
43+
// NOTE: When using this type, reject fractional numbers using a Pattern to
44+
// avoid an upstream bug: https://github.com/kubernetes/kube-openapi/issues/523
45+
//
4346
// [defined by OpenAPI]: https://spec.openapis.org/registry/format/duration.html
4447
// [format]: https://spec.openapis.org/oas/latest.html#data-type-format
4548
type Duration struct {

pkg/apis/postgres-operator.crunchydata.com/v1beta1/shared_types_test.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,18 @@ func TestDurationYAML(t *testing.T) {
139139
assert.Assert(t, !strfmt.IsDuration(tt))
140140
}
141141
})
142+
143+
t.Run("DoNotUsePartialAmounts", func(t *testing.T) {
144+
var parsed Duration
145+
assert.NilError(t, yaml.Unmarshal([]byte(`1.5 hours`), &parsed))
146+
147+
expected, err := time.ParseDuration(`1.5h`)
148+
assert.NilError(t, err)
149+
150+
// The parsed value is *not* the expected amount.
151+
assert.Assert(t, parsed.AsDuration().Duration != expected,
152+
"expected https://github.com/kubernetes/kube-openapi/issues/523")
153+
})
142154
}
143155

144156
func TestSchemalessObjectDeepCopy(t *testing.T) {

0 commit comments

Comments
 (0)