From a9d6266a9224475d614adf8632e58cbe903fd2c9 Mon Sep 17 00:00:00 2001 From: Kit Patella Date: Wed, 25 Sep 2024 14:54:53 -0700 Subject: [PATCH] fix: address some fixmes before merge Signed-off-by: Kit Patella --- src/cmd/destroy.go | 2 +- src/cmd/initialize.go | 6 +++--- src/pkg/utils/cosign.go | 3 +-- src/test/external/ext_in_cluster_test.go | 9 ++++++--- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/cmd/destroy.go b/src/cmd/destroy.go index 427292bd5e..3c051b9a5e 100644 --- a/src/cmd/destroy.go +++ b/src/cmd/destroy.go @@ -31,7 +31,7 @@ var destroyCmd = &cobra.Command{ Aliases: []string{"d"}, Short: lang.CmdDestroyShort, Long: lang.CmdDestroyLong, - // FIXME(mkcp): This function deeply needs a refactor and un-nesting. + // TODO(mkcp): refactor and de-nest this function. RunE: func(cmd *cobra.Command, _ []string) error { ctx := cmd.Context() timeoutCtx, cancel := context.WithTimeout(cmd.Context(), cluster.DefaultTimeout) diff --git a/src/cmd/initialize.go b/src/cmd/initialize.go index 9dd680c259..3b9f10c1f6 100644 --- a/src/cmd/initialize.go +++ b/src/cmd/initialize.go @@ -99,10 +99,10 @@ func findInitPackage(ctx context.Context, initPackageName string) (string, error return "", err } // Verify that we can write to the path - // FIXME(mkcp): Decompose this into a helper function if helpers.InvalidPath(absCachePath) { // Create the directory if the path is invalid - if err := helpers.CreateDirectory(absCachePath, helpers.ReadExecuteAllWriteUser); err != nil { + err = helpers.CreateDirectory(absCachePath, helpers.ReadExecuteAllWriteUser) + if err != nil { return "", fmt.Errorf("unable to create the cache directory %s: %w", absCachePath, err) } } @@ -138,7 +138,7 @@ func downloadInitPackage(ctx context.Context, cacheDirectory string) (string, er message.Note(lang.CmdInitPullNote) // Prompt the user if --confirm not specified - // FIXME(mkcp): This condition can never be met + // REVIEW(mkcp): It looks like this condition can't be met - maybe --confirm was removed at some point? if !confirmDownload { prompt := &survey.Confirm{ Message: lang.CmdInitPullConfirm, diff --git a/src/pkg/utils/cosign.go b/src/pkg/utils/cosign.go index f8c82bc69c..0ecc1c8986 100644 --- a/src/pkg/utils/cosign.go +++ b/src/pkg/utils/cosign.go @@ -231,8 +231,7 @@ func GetCosignArtifacts(image string) ([]string, error) { return nil, err } - // FIXME(mkcp): Re-review how this function is used and whether a signed entity err-miss is something we should pass - // up to the caller. Needs a comment if nothing else. + // Return empty if we don't have a signature on the image var remoteOpts []ociremote.Option simg, _ := ociremote.SignedEntity(ref, remoteOpts...) if simg == nil { diff --git a/src/test/external/ext_in_cluster_test.go b/src/test/external/ext_in_cluster_test.go index 91b257651c..99984634d8 100644 --- a/src/test/external/ext_in_cluster_test.go +++ b/src/test/external/ext_in_cluster_test.go @@ -201,9 +201,12 @@ func (suite *ExtInClusterTestSuite) Test_1_Deploy() { suite.NoError(err, "unable to teardown zarf") // FIXME(mkcp): This code always fails to clean up the tmpdir. We're passing it a directory with contents and we - // need os.RemoveAll(temp) instead. However, we're also getting what looks to be a bug with concurrent writers - // causing a subtest to panic when we call RemoveAll(). Fixing this would require removing the shared state. - _ = os.Remove(temp) //nolint:errcheck + // need os.RemoveAll(temp) instead. However, we're also getting what looks to be a bug with concurrent + // writers causing a subtest to panic when we call RemoveAll(). Fixing this would require removing the + // shared state. + // REVIEW(mkcp): Thinking we delete this and + err = os.RemoveAll(temp) //nolint:errcheck + suite.NoError(err, "failed to clean up tempdir") } func TestExtInClusterTestSuite(t *testing.T) {