From 543089cde3aa102487cd0926b68dff44aa5267af Mon Sep 17 00:00:00 2001 From: Nathan Rijksen Date: Fri, 8 Nov 2024 15:00:09 -0800 Subject: [PATCH] Fix time comparison issues with buildscripts due to inconsistent time formatting --- internal/runbits/buildscript/buildscript_test.go | 2 +- pkg/buildscript/buildscript.go | 12 +++++++++++- pkg/buildscript/buildscript_test.go | 13 ++++++------- pkg/buildscript/marshal.go | 4 ++-- pkg/buildscript/merge_test.go | 4 ++-- pkg/buildscript/raw_test.go | 6 +++--- pkg/buildscript/unmarshal.go | 12 ++++++++---- pkg/buildscript/unmarshal_buildexpression.go | 2 +- scripts/to-buildscript/main.go | 3 +-- 9 files changed, 35 insertions(+), 23 deletions(-) diff --git a/internal/runbits/buildscript/buildscript_test.go b/internal/runbits/buildscript/buildscript_test.go index 24e47de8ef..60190339ee 100644 --- a/internal/runbits/buildscript/buildscript_test.go +++ b/internal/runbits/buildscript/buildscript_test.go @@ -12,7 +12,7 @@ import ( ) const testProject = "https://platform.activestate.com/org/project?branch=main&commitID=00000000-0000-0000-0000-000000000000" -const testTime = "2000-01-01T00:00:00.000Z" +const testTime = "2000-01-01T00:00:00Z" func checkoutInfo(project, time string) string { return "```\n" + diff --git a/pkg/buildscript/buildscript.go b/pkg/buildscript/buildscript.go index e3a8ed3d6b..9909f80527 100644 --- a/pkg/buildscript/buildscript.go +++ b/pkg/buildscript/buildscript.go @@ -7,6 +7,7 @@ import ( "github.com/ActiveState/cli/internal/condition" "github.com/ActiveState/cli/internal/errs" + "github.com/ActiveState/cli/internal/logging" "github.com/ActiveState/cli/internal/rtutils/ptr" "github.com/ActiveState/cli/internal/sliceutils" "github.com/brunoga/deep" @@ -65,7 +66,15 @@ func (b *BuildScript) SetAtTime(t time.Time, override bool) { if b.atTime != nil && !override { return } - b.atTime = &t + // Ensure time is RFC3339 formatted, even though it's not actually stored at that format, it does still + // affect the specificity of the data stored and can ultimately lead to inconsistencies if not explicitly handled. + t2, err := time.Parse(time.RFC3339, t.Format(time.RFC3339)) + if err != nil { + // Pointless error check as this should never happen, but you know what they say about assumptions + logging.Error("Error parsing time: %s", err) + } + b.atTime = ptr.To(t2) + _ = b.atTime } func (b *BuildScript) Equals(other *BuildScript) (bool, error) { @@ -90,6 +99,7 @@ func (b *BuildScript) Equals(other *BuildScript) (bool, error) { if err != nil { return false, errs.Wrap(err, "Unable to marshal other buildscript") } + return string(myBytes) == string(otherBytes), nil } diff --git a/pkg/buildscript/buildscript_test.go b/pkg/buildscript/buildscript_test.go index 4bc18a2d06..2d0406833c 100644 --- a/pkg/buildscript/buildscript_test.go +++ b/pkg/buildscript/buildscript_test.go @@ -8,7 +8,6 @@ import ( "github.com/ActiveState/cli/internal/environment" "github.com/ActiveState/cli/internal/fileutils" - "github.com/go-openapi/strfmt" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -118,7 +117,7 @@ func TestRoundTripFromBuildExpressionWithLegacyAtTime(t *testing.T) { // Ensure that legacy at_time is preserved in the buildscript. atTime := script.AtTime() require.NotNil(t, atTime) - require.Equal(t, initialTimeStamp, atTime.Format(strfmt.RFC3339Millis)) + require.Equal(t, initialTimeStamp, atTime.Format(time.RFC3339)) data, err = script.MarshalBuildExpression() require.NoError(t, err) @@ -128,12 +127,12 @@ func TestRoundTripFromBuildExpressionWithLegacyAtTime(t *testing.T) { assert.NotContains(t, string(data), initialTimeStamp) // Update the time in the build script but don't override the existing time - updatedTime, err := time.Parse(strfmt.RFC3339Millis, updatedTimeStamp) + updatedTime, err := time.Parse(time.RFC3339, updatedTimeStamp) require.NoError(t, err) script.SetAtTime(updatedTime, false) // The updated time should be reflected in the build script - require.Equal(t, initialTimeStamp, script.AtTime().Format(strfmt.RFC3339Millis)) + require.Equal(t, initialTimeStamp, script.AtTime().Format(time.RFC3339)) data, err = script.Marshal() require.NoError(t, err) @@ -145,7 +144,7 @@ func TestRoundTripFromBuildExpressionWithLegacyAtTime(t *testing.T) { // Now override the time in the build script script.SetAtTime(updatedTime, true) - require.Equal(t, updatedTimeStamp, script.AtTime().Format(strfmt.RFC3339Millis)) + require.Equal(t, updatedTimeStamp, script.AtTime().Format(time.RFC3339)) data, err = script.Marshal() require.NoError(t, err) @@ -165,7 +164,7 @@ func TestRoundTripFromBuildExpressionWithLegacyAtTime(t *testing.T) { // TestExpressionToScript tests that creating a build script from a given Platform build expression // and at time produces the expected result. func TestExpressionToScript(t *testing.T) { - ts, err := time.Parse(strfmt.RFC3339Millis, testTime) + ts, err := time.Parse(time.RFC3339, testTime) require.NoError(t, err) script := New() @@ -193,7 +192,7 @@ func TestScriptToExpression(t *testing.T) { func TestOutdatedScript(t *testing.T) { _, err := Unmarshal([]byte( - `at_time = "2000-01-01T00:00:00.000Z" + `at_time = "2000-01-01T00:00:00Z" main = runtime `)) assert.Error(t, err) diff --git a/pkg/buildscript/marshal.go b/pkg/buildscript/marshal.go index 6eefd259f8..4a006a5f55 100644 --- a/pkg/buildscript/marshal.go +++ b/pkg/buildscript/marshal.go @@ -5,8 +5,8 @@ import ( "fmt" "strconv" "strings" + "time" - "github.com/go-openapi/strfmt" "github.com/thoas/go-funk" ) @@ -31,7 +31,7 @@ func (b *BuildScript) Marshal() ([]byte, error) { buf.WriteString("```\n") buf.WriteString("Project: " + b.project + "\n") if b.atTime != nil { - buf.WriteString("Time: " + b.atTime.Format(strfmt.RFC3339Millis) + "\n") + buf.WriteString("Time: " + b.atTime.Format(time.RFC3339) + "\n") } buf.WriteString("```\n\n") diff --git a/pkg/buildscript/merge_test.go b/pkg/buildscript/merge_test.go index 260bb50867..5dddb14fd8 100644 --- a/pkg/buildscript/merge_test.go +++ b/pkg/buildscript/merge_test.go @@ -8,8 +8,8 @@ import ( "github.com/stretchr/testify/require" ) -const mergeATime = "2000-01-01T00:00:00.000Z" -const mergeBTime = "2000-01-02T00:00:00.000Z" +const mergeATime = "2000-01-01T00:00:00Z" +const mergeBTime = "2000-01-02T00:00:00Z" func TestMergeAdd(t *testing.T) { scriptA, err := Unmarshal([]byte( diff --git a/pkg/buildscript/raw_test.go b/pkg/buildscript/raw_test.go index 306a71dbbe..a19a7e3f55 100644 --- a/pkg/buildscript/raw_test.go +++ b/pkg/buildscript/raw_test.go @@ -11,7 +11,7 @@ import ( ) const testProject = "https://platform.activestate.com/org/project?branch=main&commitID=00000000-0000-0000-0000-000000000000" -const testTime = "2000-01-01T00:00:00.000Z" +const testTime = "2000-01-01T00:00:00Z" func checkoutInfoString(project, time string) string { return "```\n" + @@ -43,7 +43,7 @@ main = runtime `)) require.NoError(t, err) - atTimeStrfmt, err := strfmt.ParseDateTime("2000-01-01T00:00:00.000Z") + atTimeStrfmt, err := strfmt.ParseDateTime("2000-01-01T00:00:00Z") require.NoError(t, err) atTime := time.Time(atTimeStrfmt) @@ -121,7 +121,7 @@ main = merge( `)) require.NoError(t, err) - atTimeStrfmt, err := strfmt.ParseDateTime("2000-01-01T00:00:00.000Z") + atTimeStrfmt, err := strfmt.ParseDateTime("2000-01-01T00:00:00Z") require.NoError(t, err) atTime := time.Time(atTimeStrfmt) diff --git a/pkg/buildscript/unmarshal.go b/pkg/buildscript/unmarshal.go index 9602aef76c..5382e135e5 100644 --- a/pkg/buildscript/unmarshal.go +++ b/pkg/buildscript/unmarshal.go @@ -12,7 +12,6 @@ import ( "github.com/ActiveState/cli/internal/constants" "github.com/ActiveState/cli/internal/errs" "github.com/ActiveState/cli/internal/locale" - "github.com/ActiveState/cli/internal/rtutils/ptr" ) const atTimeKey = "at_time" @@ -72,11 +71,16 @@ func Unmarshal(data []byte) (*BuildScript, error) { project = info.Project - atTimeStr, err := strfmt.ParseDateTime(info.Time) + atTimeVal, err := time.Parse(time.RFC3339, info.Time) if err != nil { - return nil, errs.Wrap(err, "Invalid timestamp: %s", info.Time) + // Older buildscripts used microsecond specificity + atDateTime, err := strfmt.ParseDateTime(info.Time) + if err != nil { + return nil, errs.Wrap(err, "Invalid timestamp: %s", info.Time) + } + atTimeVal = time.Time(atDateTime) } - atTime = ptr.To(time.Time(atTimeStr)) + atTime = &atTimeVal } return &BuildScript{raw, project, atTime}, nil diff --git a/pkg/buildscript/unmarshal_buildexpression.go b/pkg/buildscript/unmarshal_buildexpression.go index 8561deb229..0323da3514 100644 --- a/pkg/buildscript/unmarshal_buildexpression.go +++ b/pkg/buildscript/unmarshal_buildexpression.go @@ -78,7 +78,7 @@ func (b *BuildScript) UnmarshalBuildExpression(data []byte) error { atTimeNode.Str = nil atTimeNode.Ident = ptr.To("TIME") // Preserve the original at_time found in the solve node. - b.atTime = ptr.To(time.Time(atTime)) + b.SetAtTime(time.Time(atTime), true) } else if atTimeNode.Ident != nil && *atTimeNode.Ident == "at_time" { atTimeNode.Ident = ptr.To("TIME") } diff --git a/scripts/to-buildscript/main.go b/scripts/to-buildscript/main.go index 84a2e4599f..6800ddd78a 100644 --- a/scripts/to-buildscript/main.go +++ b/scripts/to-buildscript/main.go @@ -10,7 +10,6 @@ import ( "github.com/ActiveState/cli/internal/errs" "github.com/ActiveState/cli/internal/fileutils" "github.com/ActiveState/cli/pkg/buildscript" - "github.com/go-openapi/strfmt" ) func main() { @@ -45,7 +44,7 @@ func main() { project := "https://platform.activestate.com/org/project?branch=main&commitID=00000000-0000-0000-0000-000000000000" var atTime *time.Time if len(os.Args) == (2 + argOffset) { - t, err := time.Parse(strfmt.RFC3339Millis, os.Args[1+argOffset]) + t, err := time.Parse(time.RFC3339, os.Args[1+argOffset]) if err != nil { panic(errs.JoinMessage(err)) }