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 29, 2024
1 parent ea28ea2 commit 6fcbe7d
Show file tree
Hide file tree
Showing 41 changed files with 155 additions and 156 deletions.
13 changes: 8 additions & 5 deletions src/cmd/destroy.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,16 @@ 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/message"
"github.com/zarf-dev/zarf/src/pkg/logging"
"github.com/zarf-dev/zarf/src/pkg/utils/exec"

"github.com/spf13/cobra"
)

var confirmDestroy bool
Expand All @@ -32,6 +33,8 @@ var destroyCmd = &cobra.Command{
Long: lang.CmdDestroyLong,
RunE: func(cmd *cobra.Command, _ []string) error {
ctx := cmd.Context()
log := logging.FromContextOrDiscard(ctx)

timeoutCtx, cancel := context.WithTimeout(cmd.Context(), cluster.DefaultTimeout)
defer cancel()
c, err := cluster.NewClusterWithWait(timeoutCtx)
Expand All @@ -44,7 +47,7 @@ var destroyCmd = &cobra.Command{
// the scripts to remove k3s, we will still try to remove a locally installed k3s cluster
state, err := c.LoadZarfState(ctx)
if err != nil {
message.WarnErr(err, err.Error())
log.Warn("could not load Zarf state", "error", err)
}

// If Zarf deployed the cluster, burn it all down
Expand All @@ -63,7 +66,7 @@ var destroyCmd = &cobra.Command{
// Run the matched script
err := exec.CmdWithPrint(script)
if errors.Is(err, os.ErrPermission) {
message.Warnf(lang.CmdDestroyErrScriptPermissionDenied, script)
log.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
3 changes: 2 additions & 1 deletion src/cmd/internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/zarf-dev/zarf/src/internal/agent"
"github.com/zarf-dev/zarf/src/internal/gitea"
"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/types"
)
Expand Down Expand Up @@ -336,7 +337,7 @@ var updateGiteaPVC = &cobra.Command{
// There is a possibility that the pvc does not yet exist and Gitea helm chart should create it
helmShouldCreate, err := c.UpdateGiteaPVC(ctx, pvcName, rollback)
if err != nil {
message.WarnErr(err, lang.CmdInternalUpdateGiteaPVCErr)
logging.FromContextOrDiscard(ctx).Warn("Unable to update the existing Gitea persistent volume claim.", "error", err)
}
fmt.Print(helmShouldCreate)
return 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
10 changes: 6 additions & 4 deletions src/cmd/tools/zarf.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,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 @@ -40,8 +41,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 @@ -101,6 +102,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 @@ -179,7 +181,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() && internalGitServerExists {
Expand All @@ -192,7 +194,7 @@ var updateCredsCmd = &cobra.Command{
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
22 changes: 4 additions & 18 deletions src/config/lang/english.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,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 @@ -101,8 +98,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 @@ -202,7 +197,6 @@ $ zarf init --artifact-push-password={PASSWORD} --artifact-push-username={USERNA
CmdInternalUpdateGiteaPVCShort = "Updates an existing Gitea persistent volume claim"
CmdInternalUpdateGiteaPVCLong = "Updates an existing Gitea persistent volume claim by assessing if claim is a custom user provided claim or default." +
"This is called internally by the supported Gitea package component."
CmdInternalUpdateGiteaPVCErr = "Unable to update the existing Gitea persistent volume claim."
CmdInternalFlagUpdateGiteaPVCRollback = "Roll back previous Gitea persistent volume claim updates."

CmdInternalIsValidHostnameShort = "Checks if the current machine's hostname is RFC1123 compliant"
Expand Down Expand Up @@ -267,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 @@ -332,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 @@ -429,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 @@ -623,8 +614,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
Loading

0 comments on commit 6fcbe7d

Please sign in to comment.