Skip to content

Commit

Permalink
refactor: replace message.Warn with context logger
Browse files Browse the repository at this point in the history
Signed-off-by: Philip Laine <philip.laine@gmail.com>
  • Loading branch information
phillebaba committed Aug 7, 2024
1 parent 3ed9004 commit 032b78e
Show file tree
Hide file tree
Showing 37 changed files with 139 additions and 126 deletions.
8 changes: 5 additions & 3 deletions src/cmd/destroy.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,17 @@ import (
"os"
"regexp"

"github.com/spf13/cobra"

"github.com/defenseunicorns/pkg/helpers/v2"

"github.com/zarf-dev/zarf/src/config"
"github.com/zarf-dev/zarf/src/config/lang"
"github.com/zarf-dev/zarf/src/internal/packager/helm"
"github.com/zarf-dev/zarf/src/pkg/cluster"
"github.com/zarf-dev/zarf/src/pkg/logging"
"github.com/zarf-dev/zarf/src/pkg/message"
"github.com/zarf-dev/zarf/src/pkg/utils/exec"

"github.com/spf13/cobra"
)

var confirmDestroy bool
Expand Down Expand Up @@ -63,7 +65,7 @@ var destroyCmd = &cobra.Command{
// Run the matched script
err := exec.CmdWithPrint(script)
if errors.Is(err, os.ErrPermission) {
message.Warnf(lang.CmdDestroyErrScriptPermissionDenied, script)
logging.FromContextOrDiscard(cmd.Context()).Warn("Received 'permission denied' when trying to execute the script (%s). Please double-check you have the correct kube-context.", "script", script)

// Don't remove scripts we can't execute so the user can try to manually run
continue
Expand Down
8 changes: 4 additions & 4 deletions src/cmd/dev.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
"github.com/zarf-dev/zarf/src/config"
"github.com/zarf-dev/zarf/src/config/lang"
"github.com/zarf-dev/zarf/src/pkg/lint"
"github.com/zarf-dev/zarf/src/pkg/message"
"github.com/zarf-dev/zarf/src/pkg/logging"
"github.com/zarf-dev/zarf/src/pkg/packager"
"github.com/zarf-dev/zarf/src/pkg/transform"
"github.com/zarf-dev/zarf/src/pkg/utils"
Expand Down Expand Up @@ -97,7 +97,7 @@ var devTransformGitLinksCmd = &cobra.Command{
Aliases: []string{"p"},
Short: lang.CmdDevPatchGitShort,
Args: cobra.ExactArgs(2),
RunE: func(_ *cobra.Command, args []string) error {
RunE: func(cmd *cobra.Command, args []string) error {
host, fileName := args[0], args[1]

// Read the contents of the given file
Expand All @@ -110,7 +110,7 @@ var devTransformGitLinksCmd = &cobra.Command{

// Perform git url transformation via regex
text := string(content)
processedText := transform.MutateGitURLsInText(message.Warnf, pkgConfig.InitOpts.GitServer.Address, text, pkgConfig.InitOpts.GitServer.PushUsername)
processedText := transform.MutateGitURLsInText(cmd.Context(), pkgConfig.InitOpts.GitServer.Address, text, pkgConfig.InitOpts.GitServer.PushUsername)

// Print the differences
dmp := diffmatchpatch.New()
Expand Down Expand Up @@ -153,7 +153,7 @@ var devSha256SumCmd = &cobra.Command{
var err error

if helpers.IsURL(fileName) {
message.Warn(lang.CmdDevSha256sumRemoteWarning)
logging.FromContextOrDiscard(cmd.Context()).Warn("This is a remote source. If a published checksum is available you should use that rather than calculating it directly from the remote link.")

fileBase, err := helpers.ExtractBasePathFromURL(fileName)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/package.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ var packageCreateCmd = &cobra.Command{

var isCleanPathRegex = regexp.MustCompile(`^[a-zA-Z0-9\_\-\/\.\~\\:]+$`)
if !isCleanPathRegex.MatchString(config.CommonOptions.CachePath) {
message.Warnf(lang.CmdPackageCreateCleanPathErr, config.ZarfDefaultCachePath)
logging.FromContextOrDiscard(cmd.Context()).Warn("Invalid characters in the Zarf cache path, defaulting to path", "default", config.ZarfDefaultCachePath)
config.CommonOptions.CachePath = config.ZarfDefaultCachePath
}

Expand Down
5 changes: 3 additions & 2 deletions src/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,13 @@ var rootCmd = &cobra.Command{
_, _ = fmt.Fprintln(os.Stderr, zarfLogo)
cmd.Help()

log := logging.FromContextOrDiscard(cmd.Context())
if len(args) > 0 {
if strings.Contains(args[0], config.ZarfPackagePrefix) || strings.Contains(args[0], "zarf-init") {
message.Warnf(lang.RootCmdDeprecatedDeploy, args[0])
log.Warn("Deprecated: Please use \"zarf package deploy\" to deploy this package. This warning will be removed in Zarf v1.0.0.")
}
if args[0] == layout.ZarfYAML {
message.Warn(lang.RootCmdDeprecatedCreate)
log.Warn("Deprecated: Please use \"zarf package create\" to create this package. This warning will be removed in Zarf v1.0.0.")
}
}
},
Expand Down
3 changes: 2 additions & 1 deletion src/cmd/tools/crane.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/zarf-dev/zarf/src/config/lang"
"github.com/zarf-dev/zarf/src/internal/packager/images"
"github.com/zarf-dev/zarf/src/pkg/cluster"
"github.com/zarf-dev/zarf/src/pkg/logging"
"github.com/zarf-dev/zarf/src/pkg/message"
"github.com/zarf-dev/zarf/src/pkg/transform"
"github.com/zarf-dev/zarf/src/types"
Expand Down Expand Up @@ -176,7 +177,7 @@ func zarfCraneInternalWrapper(commandToWrap func(*[]crane.Option) *cobra.Command

zarfState, err := c.LoadZarfState(ctx)
if err != nil {
message.Warnf("could not get Zarf state from Kubernetes cluster, continuing without state information %s", err.Error())
logging.FromContextOrDiscard(cmd.Context()).Warn("could not get Zarf state from Kubernetes cluster, continuing without state information", "error", err)
return originalListFn(cmd, args)
}

Expand Down
14 changes: 8 additions & 6 deletions src/cmd/tools/zarf.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/zarf-dev/zarf/src/internal/packager/helm"
"github.com/zarf-dev/zarf/src/internal/packager/template"
"github.com/zarf-dev/zarf/src/pkg/cluster"
"github.com/zarf-dev/zarf/src/pkg/logging"
"github.com/zarf-dev/zarf/src/pkg/message"
"github.com/zarf-dev/zarf/src/pkg/packager/sources"
"github.com/zarf-dev/zarf/src/pkg/pki"
Expand All @@ -41,8 +42,8 @@ var deprecatedGetGitCredsCmd = &cobra.Command{
Hidden: true,
Short: lang.CmdToolsGetGitPasswdShort,
Long: lang.CmdToolsGetGitPasswdLong,
Run: func(_ *cobra.Command, _ []string) {
message.Warn(lang.CmdToolsGetGitPasswdDeprecation)
Run: func(cmd *cobra.Command, _ []string) {
logging.FromContextOrDiscard(cmd.Context()).Warn("Deprecated: This command has been replaced by 'zarf tools get-creds git' and will be removed in Zarf v1.0.0.")
getCredsCmd.Run(getCredsCmd, []string{"git"})
},
}
Expand Down Expand Up @@ -102,6 +103,7 @@ var updateCredsCmd = &cobra.Command{
}

ctx := cmd.Context()
log := logging.FromContextOrDiscard(ctx)

timeoutCtx, cancel := context.WithTimeout(ctx, cluster.DefaultTimeout)
defer cancel()
Expand Down Expand Up @@ -173,7 +175,7 @@ var updateCredsCmd = &cobra.Command{
})
if err != nil {
// Warn if we couldn't actually update the git server (it might not be installed and we should try to continue)
message.Warnf(lang.CmdToolsUpdateCredsUnableCreateToken, err.Error())
log.Warn("Unable to create the new Gitea artifact token", "error", err)
}
}

Expand All @@ -190,7 +192,7 @@ var updateCredsCmd = &cobra.Command{
err = h.UpdateZarfRegistryValues(ctx)
if err != nil {
// Warn if we couldn't actually update the registry (it might not be installed and we should try to continue)
message.Warnf(lang.CmdToolsUpdateCredsUnableUpdateRegistry, err.Error())
log.Warn("Unable to update Zarf Registry values", "error", err)
}
}
if slices.Contains(args, message.GitKey) && newState.GitServer.IsInternal() {
Expand Down Expand Up @@ -221,14 +223,14 @@ var updateCredsCmd = &cobra.Command{
})
if err != nil {
// Warn if we couldn't actually update the git server (it might not be installed and we should try to continue)
message.Warnf(lang.CmdToolsUpdateCredsUnableUpdateGit, err.Error())
log.Warn("Unable to update Zarf Git Server values", "error", err)
}
}
if slices.Contains(args, message.AgentKey) {
err = h.UpdateZarfAgentValues(ctx)
if err != nil {
// Warn if we couldn't actually update the agent (it might not be installed and we should try to continue)
message.Warnf(lang.CmdToolsUpdateCredsUnableUpdateAgent, err.Error())
log.Warn("Unable to update Zarf Agent TLS secrets", "error", err)
}
}
}
Expand Down
21 changes: 4 additions & 17 deletions src/config/lang/english.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,6 @@ const (
RootCmdFlagTempDir = "Specify the temporary directory to use for intermediate files"
RootCmdFlagInsecure = "Allow access to insecure registries and disable other recommended security enforcements such as package checksum and signature validation. This flag should only be used if you have a specific reason and accept the reduced security posture."

RootCmdDeprecatedDeploy = "Deprecated: Please use \"zarf package deploy %s\" to deploy this package. This warning will be removed in Zarf v1.0.0."
RootCmdDeprecatedCreate = "Deprecated: Please use \"zarf package create\" to create this package. This warning will be removed in Zarf v1.0.0."

// zarf connect
CmdConnectShort = "Accesses services or pods deployed in the cluster"
CmdConnectLong = "Uses a k8s port-forward to connect to resources within the cluster referenced by your kube-context.\n" +
Expand Down Expand Up @@ -100,8 +97,6 @@ const (
CmdDestroyFlagConfirm = "REQUIRED. Confirm the destroy action to prevent accidental deletions"
CmdDestroyFlagRemoveComponents = "Also remove any installed components outside the zarf namespace"

CmdDestroyErrScriptPermissionDenied = "Received 'permission denied' when trying to execute the script (%s). Please double-check you have the correct kube-context."

// zarf init
CmdInitShort = "Prepares a k8s cluster for the deployment of Zarf packages"
CmdInitLong = "Injects an OCI registry as well as an optional git server " +
Expand Down Expand Up @@ -266,7 +261,6 @@ $ zarf package mirror-resources <your-package.tar.zst> \
CmdPackageCreateFlagDifferential = "[beta] Build a package that only contains the differential changes from local resources and differing remote resources from the specified previously built package"
CmdPackageCreateFlagRegistryOverride = "Specify a map of domains to override on package create when pulling images (e.g. --registry-override docker.io=dockerio-reg.enterprise.intranet)"
CmdPackageCreateFlagFlavor = "The flavor of components to include in the resulting package (i.e. have a matching or empty \"only.flavor\" key)"
CmdPackageCreateCleanPathErr = "Invalid characters in Zarf cache path, defaulting to %s"

CmdPackageDeployFlagConfirm = "Confirms package deployment without prompting. ONLY use with packages you trust. Skips prompts to review SBOM, configure variables, select optional components and review potential breaking changes."
CmdPackageDeployFlagAdoptExistingResources = "Adopts any pre-existing K8s resources into the Helm charts managed by Zarf. ONLY use when you have existing deployments you want Zarf to takeover."
Expand Down Expand Up @@ -331,8 +325,7 @@ $ zarf package pull oci://ghcr.io/defenseunicorns/packages/dos-games:1.0.0 -a sk
"This should only be used for manifests that are not mutated by the Zarf Agent Mutating Webhook."
CmdDevPatchGitOverwritePrompt = "Overwrite the file %s with these changes?"

CmdDevSha256sumShort = "Generates a SHA256SUM for the given file"
CmdDevSha256sumRemoteWarning = "This is a remote source. If a published checksum is available you should use that rather than calculating it directly from the remote link."
CmdDevSha256sumShort = "Generates a SHA256SUM for the given file"

CmdDevFindImagesShort = "Evaluates components in a Zarf file to identify images specified in their helm charts and manifests"
CmdDevFindImagesLong = "Evaluates components in a Zarf file to identify images specified in their helm charts and manifests.\n\n" +
Expand Down Expand Up @@ -428,10 +421,9 @@ $ zarf tools registry digest reg.example.com/stefanprodan/podinfo:6.4.0
CmdToolsRegistryFlagNonDist = "Allow pushing non-distributable (foreign) layers"
CmdToolsRegistryFlagPlatform = "Specifies the platform in the form os/arch[/variant][:osversion] (e.g. linux/amd64)."

CmdToolsGetGitPasswdShort = "[Deprecated] Returns the push user's password for the Git server"
CmdToolsGetGitPasswdLong = "[Deprecated] Reads the password for a user with push access to the configured Git server in Zarf State. Note that this command has been replaced by 'zarf tools get-creds git' and will be removed in Zarf v1.0.0."
CmdToolsGetGitPasswdDeprecation = "Deprecated: This command has been replaced by 'zarf tools get-creds git' and will be removed in Zarf v1.0.0."
CmdToolsYqExample = `
CmdToolsGetGitPasswdShort = "[Deprecated] Returns the push user's password for the Git server"
CmdToolsGetGitPasswdLong = "[Deprecated] Reads the password for a user with push access to the configured Git server in Zarf State. Note that this command has been replaced by 'zarf tools get-creds git' and will be removed in Zarf v1.0.0."
CmdToolsYqExample = `
# yq defaults to 'eval' command if no command is specified. See "zarf tools yq eval --help" for more examples.
# read the "stuff" node from "myfile.yml"
Expand Down Expand Up @@ -656,8 +648,3 @@ var (
ErrUnableToCheckArch = errors.New("unable to get the configured cluster's architecture")
ErrUnableToGetPackages = errors.New("unable to load the Zarf Package data from the cluster")
)

// Collection of reusable warn messages.
var (
WarnSGetDeprecation = "Using sget to download resources is being deprecated and will removed in the v1.0.0 release of Zarf. Please publish the packages as OCI artifacts instead."
)
7 changes: 4 additions & 3 deletions src/internal/agent/hooks/argocd-application_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,14 @@
package hooks

import (
"context"
"encoding/json"
"net/http"
"testing"

"github.com/stretchr/testify/require"
"github.com/zarf-dev/zarf/src/internal/agent/http/admission"
"github.com/zarf-dev/zarf/src/internal/agent/operations"
"github.com/zarf-dev/zarf/src/test/testutil"
"github.com/zarf-dev/zarf/src/types"
v1 "k8s.io/api/admission/v1"
"k8s.io/apimachinery/pkg/runtime"
Expand All @@ -32,13 +32,14 @@ func createArgoAppAdmissionRequest(t *testing.T, op v1.Operation, argoApp *Appli
func TestArgoAppWebhook(t *testing.T) {
t.Parallel()

ctx := context.Background()
ctx := testutil.TestContext(t)

state := &types.ZarfState{GitServer: types.GitServerInfo{
Address: "https://git-server.com",
PushUsername: "a-push-user",
}}
c := createTestClientWithZarfState(ctx, t, state)
handler := admission.NewHandler().Serve(NewApplicationMutationHook(ctx, c))
handler := admission.NewHandler().Serve(ctx, NewApplicationMutationHook(ctx, c))

tests := []admissionTest{
{
Expand Down
7 changes: 4 additions & 3 deletions src/internal/agent/hooks/argocd-repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
package hooks

import (
"context"
b64 "encoding/base64"
"encoding/json"
"net/http"
Expand All @@ -13,6 +12,7 @@ import (
"github.com/stretchr/testify/require"
"github.com/zarf-dev/zarf/src/internal/agent/http/admission"
"github.com/zarf-dev/zarf/src/internal/agent/operations"
"github.com/zarf-dev/zarf/src/test/testutil"
"github.com/zarf-dev/zarf/src/types"
v1 "k8s.io/api/admission/v1"
corev1 "k8s.io/api/core/v1"
Expand All @@ -35,15 +35,16 @@ func createArgoRepoAdmissionRequest(t *testing.T, op v1.Operation, argoRepo *cor
func TestArgoRepoWebhook(t *testing.T) {
t.Parallel()

ctx := context.Background()
ctx := testutil.TestContext(t)

state := &types.ZarfState{GitServer: types.GitServerInfo{
Address: "https://git-server.com",
PushUsername: "a-push-user",
PullPassword: "a-pull-password",
PullUsername: "a-pull-user",
}}
c := createTestClientWithZarfState(ctx, t, state)
handler := admission.NewHandler().Serve(NewRepositorySecretMutationHook(ctx, c))
handler := admission.NewHandler().Serve(ctx, NewRepositorySecretMutationHook(ctx, c))

tests := []admissionTest{
{
Expand Down
7 changes: 4 additions & 3 deletions src/internal/agent/hooks/flux-gitrepo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
package hooks

import (
"context"
"encoding/json"
"net/http"
"testing"
Expand All @@ -15,6 +14,7 @@ import (
"github.com/zarf-dev/zarf/src/config"
"github.com/zarf-dev/zarf/src/internal/agent/http/admission"
"github.com/zarf-dev/zarf/src/internal/agent/operations"
"github.com/zarf-dev/zarf/src/test/testutil"
"github.com/zarf-dev/zarf/src/types"
v1 "k8s.io/api/admission/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -36,13 +36,14 @@ func createFluxGitRepoAdmissionRequest(t *testing.T, op v1.Operation, fluxGitRep
func TestFluxMutationWebhook(t *testing.T) {
t.Parallel()

ctx := context.Background()
ctx := testutil.TestContext(t)

state := &types.ZarfState{GitServer: types.GitServerInfo{
Address: "https://git-server.com",
PushUsername: "a-push-user",
}}
c := createTestClientWithZarfState(ctx, t, state)
handler := admission.NewHandler().Serve(NewGitRepositoryMutationHook(ctx, c))
handler := admission.NewHandler().Serve(ctx, NewGitRepositoryMutationHook(ctx, c))

tests := []admissionTest{
{
Expand Down
7 changes: 4 additions & 3 deletions src/internal/agent/hooks/flux-helmrepo.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,14 @@ import (
"github.com/defenseunicorns/pkg/helpers/v2"
"github.com/fluxcd/pkg/apis/meta"
flux "github.com/fluxcd/source-controller/api/v1"
v1 "k8s.io/api/admission/v1"

"github.com/zarf-dev/zarf/src/config"
"github.com/zarf-dev/zarf/src/config/lang"
"github.com/zarf-dev/zarf/src/internal/agent/operations"
"github.com/zarf-dev/zarf/src/pkg/cluster"
"github.com/zarf-dev/zarf/src/pkg/message"
"github.com/zarf-dev/zarf/src/pkg/logging"
"github.com/zarf-dev/zarf/src/pkg/transform"
v1 "k8s.io/api/admission/v1"
)

// NewHelmRepositoryMutationHook creates a new instance of the helm repo mutation hook.
Expand All @@ -43,7 +44,7 @@ func mutateHelmRepo(ctx context.Context, r *v1.AdmissionRequest, cluster *cluste

// If we see a type of helm repo other than OCI we should flag a warning and return
if strings.ToLower(src.Spec.Type) != "oci" {
message.Warnf(lang.AgentWarnNotOCIType, src.Spec.Type)
logging.FromContextOrDiscard(ctx).Warn("Skipping HelmRepo mutation because the type is not OCI", "type", src.Spec.Type)
return &operations.Result{Allowed: true}, nil
}

Expand Down
7 changes: 4 additions & 3 deletions src/internal/agent/hooks/flux-helmrepo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
package hooks

import (
"context"
"encoding/json"
"net/http"
"testing"
Expand All @@ -15,6 +14,7 @@ import (
"github.com/zarf-dev/zarf/src/config"
"github.com/zarf-dev/zarf/src/internal/agent/http/admission"
"github.com/zarf-dev/zarf/src/internal/agent/operations"
"github.com/zarf-dev/zarf/src/test/testutil"
"github.com/zarf-dev/zarf/src/types"
v1 "k8s.io/api/admission/v1"
corev1 "k8s.io/api/core/v1"
Expand All @@ -37,7 +37,8 @@ func createFluxHelmRepoAdmissionRequest(t *testing.T, op v1.Operation, fluxHelmR
func TestFluxHelmMutationWebhook(t *testing.T) {
t.Parallel()

ctx := context.Background()
ctx := testutil.TestContext(t)

state := &types.ZarfState{RegistryInfo: types.RegistryInfo{Address: "127.0.0.1:31999"}}

tests := []admissionTest{
Expand Down Expand Up @@ -167,7 +168,7 @@ func TestFluxHelmMutationWebhook(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
c := createTestClientWithZarfState(ctx, t, state)
handler := admission.NewHandler().Serve(NewHelmRepositoryMutationHook(ctx, c))
handler := admission.NewHandler().Serve(ctx, NewHelmRepositoryMutationHook(ctx, c))
if tt.svc != nil {
_, err := c.Clientset.CoreV1().Services("zarf").Create(ctx, tt.svc, metav1.CreateOptions{})
require.NoError(t, err)
Expand Down
Loading

0 comments on commit 032b78e

Please sign in to comment.