From 7a9d2a29d3866c8eeb82a892cc543e0a8fef2b74 Mon Sep 17 00:00:00 2001 From: TP Honey Date: Wed, 30 Oct 2024 16:59:02 +0000 Subject: [PATCH] (maint) add deadcode check --- .github/workflows/tests.yml | 5 ++ cmd/auth_client.go | 8 --- cmd/changes_submit_plan.go | 47 -------------- cmd/logging.go | 18 ------ cmd/theme.go | 26 -------- tfutils/plan.go | 67 ------------------- tfutils/plan_mapper.go | 48 -------------- tfutils/plan_mapper_test.go | 124 ++++++++++++++++++++++++++++++++++-- tracing/main.go | 49 -------------- 9 files changed, 125 insertions(+), 267 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index bc02ba8a..8eb139b0 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -38,6 +38,11 @@ jobs: - name: Go Init uses: ./.github/actions/go_init + - name: find deadcode + run: | + go install golang.org/x/tools/cmd/deadcode@latest + deadcode . | tee out && [ ! -s out ] + # get .golangci.yml from github.com/overmindtech/golangci-lint_config - name: Get .golangci.yml from github.com/overmindtech/golangci-lint_configs run: | diff --git a/cmd/auth_client.go b/cmd/auth_client.go index 501669e3..75eee193 100644 --- a/cmd/auth_client.go +++ b/cmd/auth_client.go @@ -11,14 +11,6 @@ import ( "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" ) -// AuthenticatedApiKeyClient Returns an apikey client that uses the auth -// embedded in the context and otel instrumentation -func AuthenticatedApiKeyClient(ctx context.Context, oi sdp.OvermindInstance) sdpconnect.ApiKeyServiceClient { - httpClient := NewAuthenticatedClient(ctx, otelhttp.DefaultClient) - log.WithContext(ctx).WithField("apiUrl", oi.ApiUrl).Debug("Connecting to overmind apikeys API (pre-authenticated)") - return sdpconnect.NewApiKeyServiceClient(httpClient, oi.ApiUrl.String()) -} - // UnauthenticatedApiKeyClient Returns an apikey client with otel instrumentation // but no authentication. Can only be used for ExchangeKeyForToken func UnauthenticatedApiKeyClient(ctx context.Context, oi sdp.OvermindInstance) sdpconnect.ApiKeyServiceClient { diff --git a/cmd/changes_submit_plan.go b/cmd/changes_submit_plan.go index df08c19f..48d46fc1 100644 --- a/cmd/changes_submit_plan.go +++ b/cmd/changes_submit_plan.go @@ -49,53 +49,6 @@ type plannedChangeGroups struct { unsupported map[string][]*sdp.MappedItemDiff } -func (g *plannedChangeGroups) NumUnsupportedChanges() int { - num := 0 - - for _, v := range g.unsupported { - num += len(v) - } - - return num -} - -func (g *plannedChangeGroups) NumSupportedChanges() int { - num := 0 - - for _, v := range g.supported { - num += len(v) - } - - return num -} - -func (g *plannedChangeGroups) MappedItemDiffs() []*sdp.MappedItemDiff { - mappedItemDiffs := make([]*sdp.MappedItemDiff, 0) - - for _, v := range g.supported { - mappedItemDiffs = append(mappedItemDiffs, v...) - } - - for _, v := range g.unsupported { - mappedItemDiffs = append(mappedItemDiffs, v...) - } - - return mappedItemDiffs -} - -// Add the specified item to the appropriate type group in the supported or unsupported section, based of whether it has a mapping query -func (g *plannedChangeGroups) Add(typ string, item *sdp.MappedItemDiff) { - groups := g.supported - if item.GetMappingQuery() == nil { - groups = g.unsupported - } - list, ok := groups[typ] - if !ok { - list = make([]*sdp.MappedItemDiff, 0) - } - groups[typ] = append(list, item) -} - func changeTitle(arg string) string { if arg != "" { // easy, return the user's choice diff --git a/cmd/logging.go b/cmd/logging.go index 5d3a5727..df40d96b 100644 --- a/cmd/logging.go +++ b/cmd/logging.go @@ -35,25 +35,7 @@ type TextStyle struct { underlying chalk.TextStyle } -func (t TextStyle) TextStyle(val string) string { - if !tty { - // Don't style if we're not in a TTY - return val - } - - return t.underlying.TextStyle(val) -} - // A type that wraps chalk.Color but adds detections for if we're in a TTY type Color struct { underlying chalk.Color } - -func (c Color) Color(val string) string { - if !tty { - // Don't style if we're not in a TTY - return val - } - - return c.underlying.Color(val) -} diff --git a/cmd/theme.go b/cmd/theme.go index 7d7dfd4e..dec84b4a 100644 --- a/cmd/theme.go +++ b/cmd/theme.go @@ -3,12 +3,10 @@ package cmd import ( _ "embed" "fmt" - "strings" "github.com/charmbracelet/glamour" "github.com/charmbracelet/glamour/ansi" "github.com/charmbracelet/lipgloss" - "github.com/muesli/reflow/wordwrap" ) // constrain the maximum terminal width to avoid readability issues with too @@ -320,14 +318,6 @@ func styleH1() lipgloss.Style { PaddingRight(2) } -func styleH2() lipgloss.Style { - return lipgloss.NewStyle(). - Foreground(ColorPalette.BgMain). - Bold(true). - PaddingLeft(2). - PaddingRight(2) -} - // markdownToString converts the markdown string to a string containing ANSI // formatting sequences with at most maxWidth visible characters per line. Set // maxWidth to zero to use the underlying library's default. @@ -353,22 +343,6 @@ func markdownToString(maxWidth int, markdown string) string { return out } -// wrap ensures that the text is wrapped to the given width and everything but -// the first line is indented by the requested amount. Consider that the current -// implementation is very naive and for large indent values, the first line -// might not be wrapped too early. -// -// Indent is ignored when the requested indent is larger than the current width. -// This is expected to only occur in edge cases, e.g. when the terminal is -// resiyed to very narrow. -func wrap(s string, width, indent int) string { - if indent > width { - indent = 0 - } - - return strings.ReplaceAll(wordwrap.String(s, width-indent), "\n", "\n"+strings.Repeat(" ", indent)) -} - func OkSymbol() string { if IsConhost() { return "OK" diff --git a/tfutils/plan.go b/tfutils/plan.go index ddfc6271..19624902 100644 --- a/tfutils/plan.go +++ b/tfutils/plan.go @@ -5,8 +5,6 @@ import ( "regexp" "strconv" "strings" - - "github.com/xiam/dig" ) // NOTE: These definitions are copied from the @@ -62,71 +60,6 @@ type ConfigModule struct { Variables variables `json:"variables,omitempty"` } -// Digs through a map using the same logic that terraform does i.e. foo.bar[0] -func TerraformDig(srcMapPtr interface{}, path string) interface{} { - // Split the path on each period - parts := strings.Split(path, ".") - - if len(parts) == 0 { - return nil - } - - // Check for an index in this section - indexMatches := indexBrackets.FindStringSubmatch(parts[0]) - - var value interface{} - - if len(indexMatches) == 0 { - // No index, just get the value - value = dig.Interface(srcMapPtr, parts[0]) - } else { - // strip the brackets - keyName := indexBrackets.ReplaceAllString(parts[0], "") - - // Get the index - index, err := strconv.Atoi(indexMatches[1]) - - if err != nil { - return nil - } - - // Get the value - arr, ok := dig.Interface(srcMapPtr, keyName).([]interface{}) - - if !ok { - return nil - } - - // Check if the index is in range - if index < 0 || index >= len(arr) { - return nil - } - - value = arr[index] - } - - if len(parts) == 1 { - return value - } else { - // Force it to another map[string]interface{} - valueMap := make(map[string]interface{}) - - if mapString, ok := value.(map[string]string); ok { - for k, v := range mapString { - valueMap[k] = v - } - } else if mapInterface, ok := value.(map[string]interface{}); ok { - valueMap = mapInterface - } else if mapAttributeValues, ok := value.(AttributeValues); ok { - valueMap = mapAttributeValues - } else { - return nil - } - - return TerraformDig(&valueMap, strings.Join(parts[1:], ".")) - } -} - var escapeRegex = regexp.MustCompile(`\${([\w\.\[\]]*)}`) // Digs for a config resource in this module or its children diff --git a/tfutils/plan_mapper.go b/tfutils/plan_mapper.go index 929bdaf0..e12b65c8 100644 --- a/tfutils/plan_mapper.go +++ b/tfutils/plan_mapper.go @@ -345,54 +345,6 @@ func isStateFile(bytes []byte) bool { return false } -// Returns the name of the provider from the config key. If the resource isn't -// in a module, the ProviderConfigKey will be something like "kubernetes", -// however if it's in a module it's be something like -// "module.something:kubernetes". In both scenarios we want to return -// "kubernetes" -func extractProviderNameFromConfigKey(providerConfigKey string) string { - sections := strings.Split(providerConfigKey, ":") - return sections[len(sections)-1] -} - -// InterpolateScope Will interpolate variables in the scope string. These -// variables can come from the following places: -// -// * `outputs` - These are the outputs from the plan -// * `values` - These are the values from the resource in question -// -// Interpolation is done using the Terraform interpolation syntax: -// https://www.terraform.io/docs/configuration/interpolation.html -func InterpolateScope(scope string, data map[string]any) (string, error) { - // Find all instances of ${} in the Scope - matches := escapeRegex.FindAllStringSubmatch(scope, -1) - - interpolated := scope - - for _, match := range matches { - // The first match is the entire string, the second match is the - // variable name - variableName := match[1] - - value := TerraformDig(&data, variableName) - - if value == nil { - return "", fmt.Errorf("variable '%v' not found", variableName) - } - - // Convert the value to a string - valueString, ok := value.(string) - - if !ok { - return "", fmt.Errorf("variable '%v' is not a string", variableName) - } - - interpolated = strings.Replace(interpolated, match[0], valueString, 1) - } - - return interpolated, nil -} - func countSensitiveValuesInConfig(m ConfigModule) int { removedSecrets := 0 for _, v := range m.Variables { diff --git a/tfutils/plan_mapper_test.go b/tfutils/plan_mapper_test.go index 2277218d..6f7666dd 100644 --- a/tfutils/plan_mapper_test.go +++ b/tfutils/plan_mapper_test.go @@ -4,11 +4,14 @@ import ( "context" "encoding/json" "fmt" + "strconv" + "strings" "testing" "github.com/overmindtech/sdp-go" "github.com/sirupsen/logrus" "github.com/stretchr/testify/require" + "github.com/xiam/dig" ) func TestWithStateFile(t *testing.T) { @@ -333,7 +336,7 @@ func TestInterpolateScope(t *testing.T) { t.Run("with no interpolation", func(t *testing.T) { t.Parallel() - result, err := InterpolateScope("foo", map[string]any{}) + result, err := interpolateScope("foo", map[string]any{}) if err != nil { t.Error(err) @@ -347,7 +350,7 @@ func TestInterpolateScope(t *testing.T) { t.Run("with a single variable", func(t *testing.T) { t.Parallel() - result, err := InterpolateScope("${outputs.overmind_kubernetes_cluster_name}", map[string]any{ + result, err := interpolateScope("${outputs.overmind_kubernetes_cluster_name}", map[string]any{ "outputs": map[string]any{ "overmind_kubernetes_cluster_name": "foo", }, @@ -365,7 +368,7 @@ func TestInterpolateScope(t *testing.T) { t.Run("with multiple variables", func(t *testing.T) { t.Parallel() - result, err := InterpolateScope("${outputs.overmind_kubernetes_cluster_name}.${values.metadata.namespace}", map[string]any{ + result, err := interpolateScope("${outputs.overmind_kubernetes_cluster_name}.${values.metadata.namespace}", map[string]any{ "outputs": map[string]any{ "overmind_kubernetes_cluster_name": "foo", }, @@ -388,7 +391,7 @@ func TestInterpolateScope(t *testing.T) { t.Run("with a variable that doesn't exist", func(t *testing.T) { t.Parallel() - _, err := InterpolateScope("${outputs.overmind_kubernetes_cluster_name}", map[string]any{}) + _, err := interpolateScope("${outputs.overmind_kubernetes_cluster_name}", map[string]any{}) if err == nil { t.Error("Expected error, got nil") @@ -632,3 +635,116 @@ func TestHandleKnownAfterApply(t *testing.T) { t.Errorf("expected data to be %v, got %v", KnownAfterApply, val) } } + +// Returns the name of the provider from the config key. If the resource isn't +// in a module, the ProviderConfigKey will be something like "kubernetes", +// however if it's in a module it's be something like +// "module.something:kubernetes". In both scenarios we want to return +// "kubernetes" +func extractProviderNameFromConfigKey(providerConfigKey string) string { + sections := strings.Split(providerConfigKey, ":") + return sections[len(sections)-1] +} + +// interpolateScope Will interpolate variables in the scope string. These +// variables can come from the following places: +// +// * `outputs` - These are the outputs from the plan +// * `values` - These are the values from the resource in question +// +// Interpolation is done using the Terraform interpolation syntax: +// https://www.terraform.io/docs/configuration/interpolation.html +func interpolateScope(scope string, data map[string]any) (string, error) { + // Find all instances of ${} in the Scope + matches := escapeRegex.FindAllStringSubmatch(scope, -1) + + interpolated := scope + + for _, match := range matches { + // The first match is the entire string, the second match is the + // variable name + variableName := match[1] + + value := terraformDig(&data, variableName) + + if value == nil { + return "", fmt.Errorf("variable '%v' not found", variableName) + } + + // Convert the value to a string + valueString, ok := value.(string) + + if !ok { + return "", fmt.Errorf("variable '%v' is not a string", variableName) + } + + interpolated = strings.Replace(interpolated, match[0], valueString, 1) + } + + return interpolated, nil +} + +// Digs through a map using the same logic that terraform does i.e. foo.bar[0] +func terraformDig(srcMapPtr interface{}, path string) interface{} { + // Split the path on each period + parts := strings.Split(path, ".") + + if len(parts) == 0 { + return nil + } + + // Check for an index in this section + indexMatches := indexBrackets.FindStringSubmatch(parts[0]) + + var value interface{} + + if len(indexMatches) == 0 { + // No index, just get the value + value = dig.Interface(srcMapPtr, parts[0]) + } else { + // strip the brackets + keyName := indexBrackets.ReplaceAllString(parts[0], "") + + // Get the index + index, err := strconv.Atoi(indexMatches[1]) + + if err != nil { + return nil + } + + // Get the value + arr, ok := dig.Interface(srcMapPtr, keyName).([]interface{}) + + if !ok { + return nil + } + + // Check if the index is in range + if index < 0 || index >= len(arr) { + return nil + } + + value = arr[index] + } + + if len(parts) == 1 { + return value + } else { + // Force it to another map[string]interface{} + valueMap := make(map[string]interface{}) + + if mapString, ok := value.(map[string]string); ok { + for k, v := range mapString { + valueMap[k] = v + } + } else if mapInterface, ok := value.(map[string]interface{}); ok { + valueMap = mapInterface + } else if mapAttributeValues, ok := value.(AttributeValues); ok { + valueMap = mapAttributeValues + } else { + return nil + } + + return terraformDig(&valueMap, strings.Join(parts[1:], ".")) + } +} diff --git a/tracing/main.go b/tracing/main.go index 4740db4a..85ff91f3 100644 --- a/tracing/main.go +++ b/tracing/main.go @@ -4,8 +4,6 @@ import ( "context" _ "embed" "fmt" - "os" - "runtime/debug" "time" "github.com/getsentry/sentry-go" @@ -191,50 +189,3 @@ func (h *UserAgentSampler) ShouldSample(parameters sdktrace.SamplingParameters) func (h *UserAgentSampler) Description() string { return "Simple Sampler based on the UserAgent of the request" } - -// LogRecoverToReturn Recovers from a panic, logs and forwards it sentry and otel, then returns -// Does nothing when there is no panic. -func LogRecoverToReturn(ctx context.Context, loc string) { - err := recover() - if err == nil { - return - } - - stack := string(debug.Stack()) - handleError(ctx, loc, err, stack) -} - -// LogRecoverToExit Recovers from a panic, logs and forwards it sentry and otel, then exits -// Does nothing when there is no panic. -func LogRecoverToExit(ctx context.Context, loc string) { - err := recover() - if err == nil { - return - } - - stack := string(debug.Stack()) - handleError(ctx, loc, err, stack) - - // ensure that errors still get sent out - ShutdownTracer() - - os.Exit(1) -} - -func handleError(ctx context.Context, loc string, err interface{}, stack string) { - msg := fmt.Sprintf("unhandled panic in %v, exiting: %v", loc, err) - - hub := sentry.CurrentHub() - if hub != nil { - hub.Recover(err) - } - - if ctx != nil { - log.WithContext(ctx).WithFields(log.Fields{"loc": loc, "stack": stack}).Error(msg) - span := trace.SpanFromContext(ctx) - span.SetAttributes(attribute.String("ovm.panic.loc", loc)) - span.SetAttributes(attribute.String("ovm.panic.stack", stack)) - } else { - log.WithFields(log.Fields{"loc": loc, "stack": stack}).Error(msg) - } -}