diff --git a/internal/evaluator/criteria.go b/internal/evaluator/criteria.go index 2180965a0..df192b189 100644 --- a/internal/evaluator/criteria.go +++ b/internal/evaluator/criteria.go @@ -19,11 +19,12 @@ package evaluator import ( "fmt" "regexp" - "time" ecc "github.com/conforma/crds/api/v1alpha1" "github.com/google/go-containerregistry/pkg/name" log "github.com/sirupsen/logrus" + + "github.com/conforma/cli/internal/timeutil" ) // contains include/exclude items @@ -179,20 +180,26 @@ func computeIncludeExclude(src ecc.Source, p ConfigProvider) (*Criteria, *Criter func collectVolatileConfigItems(items *Criteria, volatileCriteria []ecc.VolatileCriteria, p ConfigProvider) *Criteria { at := p.EffectiveTime() for _, c := range volatileCriteria { - from, err := time.Parse(time.RFC3339, c.EffectiveOn) - if err != nil { - if c.EffectiveOn != "" { - log.Warnf("unable to parse time for criteria %q, was given %q: %v", c.Value, c.EffectiveOn, err) + from := at + if c.EffectiveOn != "" { + t, err := timeutil.ParseVolatileTime(c.EffectiveOn) + if err != nil { + log.Warnf("skipping volatile criteria %q: %v", c.Value, err) + continue } - from = at + from = t } - until, err := time.Parse(time.RFC3339, c.EffectiveUntil) - if err != nil { - if c.EffectiveUntil != "" { - log.Warnf("unable to parse time for criteria %q, was given %q: %v", c.Value, c.EffectiveUntil, err) + + until := at + if c.EffectiveUntil != "" { + t, err := timeutil.ParseVolatileTime(c.EffectiveUntil) + if err != nil { + log.Warnf("skipping volatile criteria %q: %v", c.Value, err) + continue } - until = at + until = t } + if until.Compare(at) >= 0 && from.Compare(at) <= 0 { // DEPRECATED: use c.ImageDigest instead if c.ImageRef != "" { diff --git a/internal/evaluator/criteria_test.go b/internal/evaluator/criteria_test.go index a119e7b68..d13efd6ea 100644 --- a/internal/evaluator/criteria_test.go +++ b/internal/evaluator/criteria_test.go @@ -469,7 +469,7 @@ func TestCollectVolatileConfigItems(t *testing.T) { expectedSuccess: true, // Function doesn't fail, just doesn't add items }, { - name: "Warning scenario - invalid time formats", + name: "Warning scenario - invalid time formats skips item", items: &Criteria{ digestItems: make(map[string][]string), componentItems: make(map[string][]string), @@ -484,14 +484,83 @@ func TestCollectVolatileConfigItems(t *testing.T) { }, }, configProvider: &MockConfigProvider{effectiveTime: fixedTime}, + expectedItems: &Criteria{ + digestItems: make(map[string][]string), + componentItems: make(map[string][]string), + defaultItems: []string{"existing-item"}, + }, + expectedSuccess: true, // Item is skipped when timestamp is unparseable (fail-closed) + }, + { + name: "Both timestamps garbage - item skipped", + items: &Criteria{ + digestItems: make(map[string][]string), + componentItems: make(map[string][]string), + defaultItems: []string{"existing-item"}, + }, + volatileCriteria: []ecc.VolatileCriteria{ + { + Value: "garbage-item", + EffectiveOn: "garbage", + EffectiveUntil: "also-garbage", + ImageDigest: "sha256:abc123", + }, + }, + configProvider: &MockConfigProvider{effectiveTime: fixedTime}, + expectedItems: &Criteria{ + digestItems: make(map[string][]string), + componentItems: make(map[string][]string), + defaultItems: []string{"existing-item"}, + }, + expectedSuccess: true, + }, + { + name: "Date-only format accepted as fallback", + items: &Criteria{ + digestItems: make(map[string][]string), + componentItems: make(map[string][]string), + defaultItems: []string{"existing-item"}, + }, + volatileCriteria: []ecc.VolatileCriteria{ + { + Value: "date-only-item", + EffectiveOn: "2025-08-01", + EffectiveUntil: "2025-08-31", + ImageDigest: "sha256:dateonly", + }, + }, + configProvider: &MockConfigProvider{effectiveTime: fixedTime}, expectedItems: &Criteria{ digestItems: map[string][]string{ - "sha256:def456": {"partial-invalid-item"}, + "sha256:dateonly": {"date-only-item"}, + }, + componentItems: make(map[string][]string), + defaultItems: []string{"existing-item"}, + }, + expectedSuccess: true, + }, + { + name: "EffectiveOn garbage with valid EffectiveUntil - item skipped", + items: &Criteria{ + digestItems: make(map[string][]string), + componentItems: make(map[string][]string), + defaultItems: []string{"existing-item"}, + }, + volatileCriteria: []ecc.VolatileCriteria{ + { + Value: "bad-on-item", + EffectiveOn: "garbage", + EffectiveUntil: "2025-08-31T23:59:59Z", + ImageDigest: "sha256:badon", }, + }, + configProvider: &MockConfigProvider{effectiveTime: fixedTime}, + expectedItems: &Criteria{ + digestItems: make(map[string][]string), componentItems: make(map[string][]string), defaultItems: []string{"existing-item"}, }, - expectedSuccess: true, // Function handles invalid times gracefully + expectedSuccess: true, }, { name: "Component names with volatile criteria", diff --git a/internal/policy/equivalence/equivalence.go b/internal/policy/equivalence/equivalence.go index 8e491e36b..087f439ba 100644 --- a/internal/policy/equivalence/equivalence.go +++ b/internal/policy/equivalence/equivalence.go @@ -28,6 +28,9 @@ import ( ecc "github.com/conforma/crds/api/v1alpha1" "github.com/pmezard/go-difflib/difflib" + log "github.com/sirupsen/logrus" + + "github.com/conforma/cli/internal/timeutil" ) // ImageInfo represents information about an image for volatile config matching @@ -324,12 +327,22 @@ func (ec *EquivalenceChecker) getActiveVolatileMatchers(v *ecc.VolatileSourceCon func (ec *EquivalenceChecker) isVolatileMatcherActive(m ecc.VolatileCriteria) bool { if m.EffectiveOn != "" { - if t, err := time.Parse(time.RFC3339, m.EffectiveOn); err == nil && ec.effectiveTime.Before(t) { + t, err := timeutil.ParseVolatileTime(m.EffectiveOn) + if err != nil { + log.Warnf("skipping volatile matcher %q: %v", m.Value, err) + return false + } + if ec.effectiveTime.Before(t) { return false } } if m.EffectiveUntil != "" { - if t, err := time.Parse(time.RFC3339, m.EffectiveUntil); err == nil && ec.effectiveTime.After(t) { + t, err := timeutil.ParseVolatileTime(m.EffectiveUntil) + if err != nil { + log.Warnf("skipping volatile matcher %q: %v", m.Value, err) + return false + } + if ec.effectiveTime.After(t) { return false } } diff --git a/internal/policy/equivalence/equivalence_test.go b/internal/policy/equivalence/equivalence_test.go index 1599944b7..1e743a700 100644 --- a/internal/policy/equivalence/equivalence_test.go +++ b/internal/policy/equivalence/equivalence_test.go @@ -14,6 +14,8 @@ // // SPDX-License-Identifier: Apache-2.0 +//go:build unit + package equivalence import ( @@ -2043,3 +2045,98 @@ func TestVolatileConfigWithImageInfo(t *testing.T) { }) } } + +func TestIsVolatileMatcherActive(t *testing.T) { + effectiveTime := time.Date(2025, 8, 18, 12, 0, 0, 0, time.UTC) + checker := NewEquivalenceChecker(effectiveTime, nil) + + tests := []struct { + name string + matcher ecc.VolatileCriteria + expected bool + }{ + { + name: "valid timestamps in range returns true", + matcher: ecc.VolatileCriteria{ + Value: "some-rule", + EffectiveOn: "2025-08-01T00:00:00Z", + EffectiveUntil: "2025-08-31T23:59:59Z", + }, + expected: true, + }, + { + name: "empty timestamps returns true (open-ended)", + matcher: ecc.VolatileCriteria{ + Value: "open-ended-rule", + }, + expected: true, + }, + { + name: "garbage EffectiveOn returns false (fail-closed)", + matcher: ecc.VolatileCriteria{ + Value: "garbage-on", + EffectiveOn: "not-a-date", + }, + expected: false, + }, + { + name: "garbage EffectiveUntil returns false (fail-closed)", + matcher: ecc.VolatileCriteria{ + Value: "garbage-until", + EffectiveUntil: "also-garbage", + }, + expected: false, + }, + { + name: "both timestamps garbage returns false", + matcher: ecc.VolatileCriteria{ + Value: "both-garbage", + EffectiveOn: "garbage", + EffectiveUntil: "garbage", + }, + expected: false, + }, + { + name: "date-only format accepted, in range returns true", + matcher: ecc.VolatileCriteria{ + Value: "date-only-rule", + EffectiveOn: "2025-08-01", + EffectiveUntil: "2025-08-31", + }, + expected: true, + }, + { + name: "date-only format, out of range returns false", + matcher: ecc.VolatileCriteria{ + Value: "date-only-expired", + EffectiveOn: "2025-07-01", + EffectiveUntil: "2025-07-15", + }, + expected: false, + }, + { + name: "future EffectiveOn returns false", + matcher: ecc.VolatileCriteria{ + Value: "future-rule", + EffectiveOn: "2025-09-01T00:00:00Z", + EffectiveUntil: "2025-09-30T23:59:59Z", + }, + expected: false, + }, + { + name: "expired EffectiveUntil returns false", + matcher: ecc.VolatileCriteria{ + Value: "expired-rule", + EffectiveOn: "2025-07-01T00:00:00Z", + EffectiveUntil: "2025-07-31T23:59:59Z", + }, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, checker.isVolatileMatcherActive(tt.matcher)) + }) + } +} diff --git a/internal/timeutil/timeutil.go b/internal/timeutil/timeutil.go new file mode 100644 index 000000000..2edeb0559 --- /dev/null +++ b/internal/timeutil/timeutil.go @@ -0,0 +1,37 @@ +// Copyright The Conforma Contributors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +package timeutil + +import ( + "fmt" + "time" +) + +// Must match policy.DateFormat. +const DateFormat = "2006-01-02" + +// ParseVolatileTime tries RFC3339 first, then date-only ("2006-01-02") as a +// fallback, matching the convention used in policy.ParseEffectiveTime. +func ParseVolatileTime(s string) (time.Time, error) { + if t, err := time.Parse(time.RFC3339, s); err == nil { + return t, nil + } + if t, err := time.Parse(DateFormat, s); err == nil { + return t, nil + } + return time.Time{}, fmt.Errorf("unable to parse %q as RFC3339 or %s", s, DateFormat) +} diff --git a/internal/timeutil/timeutil_test.go b/internal/timeutil/timeutil_test.go new file mode 100644 index 000000000..bae4fb19f --- /dev/null +++ b/internal/timeutil/timeutil_test.go @@ -0,0 +1,82 @@ +// Copyright The Conforma Contributors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +//go:build unit + +package timeutil + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestParseVolatileTime(t *testing.T) { + tests := []struct { + name string + input string + expectError bool + expectTime time.Time + }{ + { + name: "valid RFC3339", + input: "2025-08-18T12:00:00Z", + expectError: false, + expectTime: time.Date(2025, 8, 18, 12, 0, 0, 0, time.UTC), + }, + { + name: "valid date-only falls back to YYYY-MM-DD", + input: "2025-08-18", + expectError: false, + expectTime: time.Date(2025, 8, 18, 0, 0, 0, 0, time.UTC), + }, + { + name: "garbage string returns error", + input: "not-a-date", + expectError: true, + }, + { + name: "empty string returns error", + input: "", + expectError: true, + }, + { + name: "partial date returns error", + input: "2025-08", + expectError: true, + }, + { + name: "RFC3339 with offset", + input: "2025-08-18T12:00:00+02:00", + expectError: false, + expectTime: time.Date(2025, 8, 18, 10, 0, 0, 0, time.UTC), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := ParseVolatileTime(tt.input) + if tt.expectError { + assert.Error(t, err) + } else { + require.NoError(t, err) + assert.True(t, tt.expectTime.Equal(result), "expected %v, got %v", tt.expectTime, result) + } + }) + } +}