diff --git a/.golangci.yml b/.golangci.yml index 7627a7e7..26164a9b 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -121,6 +121,15 @@ linters: min-len: 3 # minimal occurrences count to trigger, 3 by default min-occurrences: 5 + # Skip strings that are idiomatic Kubernetes/Crossplane field names or + # logger keys. Replacing them with constants makes the surrounding code + # harder to read than the literal strings, since these names are part of + # well-known wire protocols (the unstructured field path) or structured + # logging conventions. The `...` ellipsis is a one-off display separator + # that doesn't benefit from being a constant. + ignore-string-values: + - '^(apiVersion|kind|name|namespace|spec|status|claim|composite|crossplane|default|json|yaml)$' + - '^\.\.\.$' gocritic: # Enable multiple checks by tags, run `GL_DEBUG=gocritic golangci-lint` run to see all tags and checks. # Empty list by default. See https://github.com/go-critic/go-critic#usage -> section "Tags". @@ -179,6 +188,7 @@ linters: - containedctx - errcheck - forcetypeassert + - goconst - gochecknoglobals - gochecknoinits - gocognit @@ -187,6 +197,13 @@ linters: - unparam path: _test(ing)?\.go + # Exclude goconst from test helper files. They repeat string literals as + # part of building fluent mock APIs; extracting constants makes them + # harder to read than the literal strings. + - linters: + - goconst + path: testutils/ + # Ease some gocritic warnings on test files. - linters: - gocritic diff --git a/Earthfile b/Earthfile index fe36c117..cb50f7e3 100644 --- a/Earthfile +++ b/Earthfile @@ -296,12 +296,12 @@ go-test: # go-lint lints Go code. go-lint: - ARG GOLANGCI_LINT_VERSION=v2.11.4 + ARG GOLANGCI_LINT_VERSION=v2.12.2 FROM +go-modules # This cache is private because golangci-lint doesn't support concurrent runs. CACHE --id go-lint --sharing private /root/.cache/golangci-lint CACHE --id go-build --sharing shared /root/.cache/go-build - RUN curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin ${GOLANGCI_LINT_VERSION} + RUN curl -sSfL https://golangci-lint.run/install.sh | sh -s -- -b $(go env GOPATH)/bin ${GOLANGCI_LINT_VERSION} COPY .golangci.yml . COPY --dir cmd/ internal/ test/ . RUN golangci-lint run --fix diff --git a/cmd/diff/client/crossplane/base.go b/cmd/diff/client/crossplane/base.go index 8b1a31b2..e1817125 100644 --- a/cmd/diff/client/crossplane/base.go +++ b/cmd/diff/client/crossplane/base.go @@ -15,6 +15,24 @@ import ( "github.com/crossplane/crossplane-runtime/v2/pkg/logging" ) +// Crossplane API group and kind constants used across the client package. +const ( + // CrossplaneAPIExtGroup is the Crossplane API extensions group. + CrossplaneAPIExtGroup = "apiextensions.crossplane.io" + // CrossplaneAPIExtGroupV1 is the v1 group/version for Crossplane API extensions. + CrossplaneAPIExtGroupV1 = "apiextensions.crossplane.io/v1" + // CrossplaneAPIExtGroupV2 is the v2 group/version for Crossplane API extensions. + CrossplaneAPIExtGroupV2 = "apiextensions.crossplane.io/v2" + // CrossplanePkgGroup is the Crossplane packages group. + CrossplanePkgGroup = "pkg.crossplane.io" + // CompositionKind is the kind for Compositions. + CompositionKind = "Composition" + // CompositionRevisionKind is the kind for CompositionRevisions. + CompositionRevisionKind = "CompositionRevision" + // FunctionKind is the kind for Crossplane Functions. + FunctionKind = "Function" +) + // Initialize initializes all the clients in this bundle. func (c *Clients) Initialize(ctx context.Context, logger logging.Logger) error { return core.InitializeClients(ctx, logger, diff --git a/cmd/diff/client/crossplane/composition_client.go b/cmd/diff/client/crossplane/composition_client.go index c10bbcd4..452fe980 100644 --- a/cmd/diff/client/crossplane/composition_client.go +++ b/cmd/diff/client/crossplane/composition_client.go @@ -61,7 +61,7 @@ func NewCompositionClient(resourceClient kubernetes.ResourceClient, definitionCl func (c *DefaultCompositionClient) Initialize(ctx context.Context) error { c.logger.Debug("Initializing composition client") - gvks, err := c.resourceClient.GetGVKsForGroupKind(ctx, "apiextensions.crossplane.io", "Composition") + gvks, err := c.resourceClient.GetGVKsForGroupKind(ctx, CrossplaneAPIExtGroup, CompositionKind) if err != nil { return errors.Wrap(err, "cannot get Composition GVKs") } @@ -95,9 +95,9 @@ func (c *DefaultCompositionClient) ListCompositions(ctx context.Context) ([]*api // Define the composition GVK gvk := schema.GroupVersionKind{ - Group: "apiextensions.crossplane.io", + Group: CrossplaneAPIExtGroup, Version: "v1", - Kind: "Composition", + Kind: CompositionKind, } // TODO: we don't actually use our cached GVKs here -- but there's only one version of Composition @@ -141,9 +141,9 @@ func (c *DefaultCompositionClient) GetComposition(ctx context.Context, name stri // Not in cache, fetch from cluster gvk := schema.GroupVersionKind{ - Group: "apiextensions.crossplane.io", + Group: CrossplaneAPIExtGroup, Version: "v1", - Kind: "Composition", + Kind: CompositionKind, } unComp, err := c.resourceClient.GetResource(ctx, gvk, "" /* Compositions are cluster scoped */, name) @@ -490,7 +490,7 @@ func getCrossplaneRefPaths(apiVersion string, path ...string) [][]string { v2Path := append([]string{"spec", "crossplane"}, path...) switch apiVersion { - case "apiextensions.crossplane.io/v1": + case CrossplaneAPIExtGroupV1: // Crossplane v1 keeps these under spec.x return [][]string{v1Path} default: @@ -814,7 +814,7 @@ func (c *DefaultCompositionClient) FindCompositesUsingComposition(ctx context.Co func (c *DefaultCompositionClient) resourceUsesComposition(resource *un.Unstructured, compositionName string) bool { // Try both v1 and v2 paths - we use a non-v1 apiVersion to get both paths from getCrossplaneRefPaths // since getCrossplaneRefPaths returns both paths for non-v1 XRDs - for _, path := range getCrossplaneRefPaths("apiextensions.crossplane.io/v2", "compositionRef", "name") { + for _, path := range getCrossplaneRefPaths(CrossplaneAPIExtGroupV2, "compositionRef", "name") { if refName, found, _ := un.NestedString(resource.Object, path...); found && refName == compositionName { return true } diff --git a/cmd/diff/client/crossplane/composition_client_test.go b/cmd/diff/client/crossplane/composition_client_test.go index 62f61e33..271b5538 100644 --- a/cmd/diff/client/crossplane/composition_client_test.go +++ b/cmd/diff/client/crossplane/composition_client_test.go @@ -19,11 +19,6 @@ import ( var _ CompositionClient = (*tu.MockCompositionClient)(nil) -const ( - CrossplaneAPIExtGroup = "apiextensions.crossplane.io" - CrossplaneAPIExtGroupV1 = "apiextensions.crossplane.io/v1" -) - func TestDefaultCompositionClient_FindMatchingComposition(t *testing.T) { type fields struct { compositions map[string]*apiextensionsv1.Composition diff --git a/cmd/diff/client/crossplane/composition_revision_client.go b/cmd/diff/client/crossplane/composition_revision_client.go index 31826a34..17544c90 100644 --- a/cmd/diff/client/crossplane/composition_revision_client.go +++ b/cmd/diff/client/crossplane/composition_revision_client.go @@ -64,7 +64,7 @@ func NewCompositionRevisionClient(resourceClient kubernetes.ResourceClient, logg func (c *DefaultCompositionRevisionClient) Initialize(ctx context.Context) error { c.logger.Debug("Initializing composition revision client") - gvks, err := c.resourceClient.GetGVKsForGroupKind(ctx, "apiextensions.crossplane.io", "CompositionRevision") + gvks, err := c.resourceClient.GetGVKsForGroupKind(ctx, CrossplaneAPIExtGroup, CompositionRevisionKind) if err != nil { return errors.Wrap(err, "cannot get CompositionRevision GVKs") } @@ -82,9 +82,9 @@ func (c *DefaultCompositionRevisionClient) listCompositionRevisionsForCompositio // Define the composition revision GVK gvk := schema.GroupVersionKind{ - Group: "apiextensions.crossplane.io", + Group: CrossplaneAPIExtGroup, Version: "v1", - Kind: "CompositionRevision", + Kind: CompositionRevisionKind, } // Use label selector to filter server-side @@ -128,9 +128,9 @@ func (c *DefaultCompositionRevisionClient) ListCompositionRevisions(ctx context. // Define the composition revision GVK gvk := schema.GroupVersionKind{ - Group: "apiextensions.crossplane.io", + Group: CrossplaneAPIExtGroup, Version: "v1", - Kind: "CompositionRevision", + Kind: CompositionRevisionKind, } // Get all composition revisions using the resource client @@ -171,9 +171,9 @@ func (c *DefaultCompositionRevisionClient) GetCompositionRevision(ctx context.Co // Not in cache, fetch from cluster gvk := schema.GroupVersionKind{ - Group: "apiextensions.crossplane.io", + Group: CrossplaneAPIExtGroup, Version: "v1", - Kind: "CompositionRevision", + Kind: CompositionRevisionKind, } unRev, err := c.resourceClient.GetResource(ctx, gvk, "" /* CompositionRevisions are cluster scoped */, name) diff --git a/cmd/diff/client/crossplane/definition_client.go b/cmd/diff/client/crossplane/definition_client.go index 9fcba0fe..e5ac3120 100644 --- a/cmd/diff/client/crossplane/definition_client.go +++ b/cmd/diff/client/crossplane/definition_client.go @@ -71,7 +71,7 @@ func NewDefinitionClient(resourceClient kubernetes.ResourceClient, logger loggin func (c *DefaultDefinitionClient) Initialize(ctx context.Context) error { c.logger.Debug("Initializing definition client") - gvks, err := c.resourceClient.GetGVKsForGroupKind(ctx, "apiextensions.crossplane.io", CompositeResourceDefinitionKind) + gvks, err := c.resourceClient.GetGVKsForGroupKind(ctx, CrossplaneAPIExtGroup, CompositeResourceDefinitionKind) if err != nil { return errors.Wrap(err, "cannot get XRD GVKs") } diff --git a/cmd/diff/client/crossplane/environment_client.go b/cmd/diff/client/crossplane/environment_client.go index abcdf762..5af9d994 100644 --- a/cmd/diff/client/crossplane/environment_client.go +++ b/cmd/diff/client/crossplane/environment_client.go @@ -46,7 +46,7 @@ func NewEnvironmentClient(resourceClient kubernetes.ResourceClient, logger loggi func (c *DefaultEnvironmentClient) Initialize(ctx context.Context) error { c.logger.Debug("Initializing environment client") - gvks, err := c.resourceClient.GetGVKsForGroupKind(ctx, "apiextensions.crossplane.io", "EnvironmentConfig") + gvks, err := c.resourceClient.GetGVKsForGroupKind(ctx, CrossplaneAPIExtGroup, "EnvironmentConfig") if err != nil { return errors.Wrap(err, "cannot get EnvironmentConfig GVKs") } diff --git a/cmd/diff/client/crossplane/function_client.go b/cmd/diff/client/crossplane/function_client.go index f947fe91..a2f85065 100644 --- a/cmd/diff/client/crossplane/function_client.go +++ b/cmd/diff/client/crossplane/function_client.go @@ -50,7 +50,7 @@ func NewFunctionClient(resourceClient kubernetes.ResourceClient, logger logging. func (c *DefaultFunctionClient) Initialize(ctx context.Context) error { c.logger.Debug("Initializing function client") - gvks, err := c.resourceClient.GetGVKsForGroupKind(ctx, "apiextensions.crossplane.io", "Function") + gvks, err := c.resourceClient.GetGVKsForGroupKind(ctx, CrossplanePkgGroup, FunctionKind) if err != nil { return errors.Wrap(err, "cannot get Function GVKs") } @@ -82,9 +82,9 @@ func (c *DefaultFunctionClient) ListFunctions(ctx context.Context) ([]pkgv1.Func // Define the function GVK gvk := schema.GroupVersionKind{ - Group: "pkg.crossplane.io", + Group: CrossplanePkgGroup, Version: "v1", - Kind: "Function", + Kind: FunctionKind, } // Get all functions using the resource client diff --git a/cmd/diff/client/kubernetes/schema_client.go b/cmd/diff/client/kubernetes/schema_client.go index 3337f806..fde08364 100644 --- a/cmd/diff/client/kubernetes/schema_client.go +++ b/cmd/diff/client/kubernetes/schema_client.go @@ -23,6 +23,14 @@ import ( xpextv2 "github.com/crossplane/crossplane/apis/v2/apiextensions/v2" ) +// XRD apiVersion strings recognized by the schema client. These mirror the +// runtime SchemeGroupVersion values from the imported xpextv1/xpextv2 packages, +// but as constants they can be used in switch cases. +const ( + xrdAPIVersionV1 = "apiextensions.crossplane.io/v1" + xrdAPIVersionV2 = "apiextensions.crossplane.io/v2" +) + // SchemaClient handles operations related to Kubernetes schemas and CRDs. type SchemaClient interface { // GetCRD gets the CustomResourceDefinition for a given GVK @@ -207,9 +215,9 @@ func extractGVKsFromXRD(xrd *un.Unstructured) ([]schema.GroupVersionKind, error) apiVersion := xrd.GetAPIVersion() switch apiVersion { - case "apiextensions.crossplane.io/v1": + case xrdAPIVersionV1: return extractGVKsFromV1XRD(xrd) - case "apiextensions.crossplane.io/v2": + case xrdAPIVersionV2: return extractGVKsFromV2XRD(xrd) default: return nil, errors.Errorf("unsupported XRD apiVersion %s in XRD %s", apiVersion, xrd.GetName()) diff --git a/cmd/diff/cmd_utils.go b/cmd/diff/cmd_utils.go index 17b2bf47..c8de2c1b 100644 --- a/cmd/diff/cmd_utils.go +++ b/cmd/diff/cmd_utils.go @@ -66,12 +66,15 @@ func defaultProcessorOptions(fields CommonCmdFields, namespace string) []dp.Proc // Import renderer package to use OutputFormat type var outputFormat renderer.OutputFormat - switch fields.Output { - case "json": + switch renderer.OutputFormat(fields.Output) { + case renderer.OutputFormatJSON: outputFormat = renderer.OutputFormatJSON - case "yaml": + case renderer.OutputFormatYAML: outputFormat = renderer.OutputFormatYAML + case renderer.OutputFormatDiff: + outputFormat = renderer.OutputFormatDiff default: + // Empty string or unrecognized values fall back to the human-readable diff. outputFormat = renderer.OutputFormatDiff } diff --git a/cmd/diff/diffprocessor/comp_processor.go b/cmd/diff/diffprocessor/comp_processor.go index 9703e9cf..01b6abb1 100644 --- a/cmd/diff/diffprocessor/comp_processor.go +++ b/cmd/diff/diffprocessor/comp_processor.go @@ -507,7 +507,7 @@ func (p *DefaultCompDiffProcessor) filterXRsByUpdatePolicy(xrs []*un.Unstructure "policy", policy) // Include XRs that are not explicitly set to Manual (i.e., Automatic or empty/default) - if policy != "Manual" { + if policy != compositionUpdatePolicyManual { filtered = append(filtered, xr) } } @@ -532,7 +532,7 @@ func (p *DefaultCompDiffProcessor) getCompositionUpdatePolicy(xr *un.Unstructure } // Default to Automatic if not found (matching Crossplane default behavior) - return "Automatic" + return compositionUpdatePolicyAutomatic } // buildImpactAnalysis builds the impact analysis and summary from XR results. diff --git a/cmd/diff/diffprocessor/diff_processor.go b/cmd/diff/diffprocessor/diff_processor.go index ad1bbf9e..b61824de 100644 --- a/cmd/diff/diffprocessor/diff_processor.go +++ b/cmd/diff/diffprocessor/diff_processor.go @@ -43,6 +43,10 @@ const ( fieldWriteConnectionSecretToRef = "writeConnectionSecretToRef" fieldCompositionRevisionRef = "compositionRevisionRef" fieldCompositionUpdatePolicy = "compositionUpdatePolicy" + + // Composition update policy values, mirroring Crossplane's CompositionUpdatePolicy. + compositionUpdatePolicyManual = "Manual" + compositionUpdatePolicyAutomatic = "Automatic" ) // DiffProcessor interface for processing resources. @@ -729,7 +733,7 @@ func buildMergedSpec(claimSpecMap, xrSpecMap map[string]any, xrForRendering *cmp // (allowing Crossplane to select the latest revision). if _, existsInClaim := claimSpecMap[fieldCompositionRevisionRef]; !existsInClaim { updatePolicy := getCompositionUpdatePolicy(xrForRendering) - if updatePolicy == "Manual" { + if updatePolicy == compositionUpdatePolicyManual { if val, exists := xrSpecMap[fieldCompositionRevisionRef]; exists { mergedSpec[fieldCompositionRevisionRef] = val } @@ -1642,5 +1646,5 @@ func getCompositionUpdatePolicy(xr *cmp.Unstructured) string { } // Default to Automatic if not found (matching Crossplane default behavior) - return "Automatic" + return compositionUpdatePolicyAutomatic } diff --git a/cmd/diff/diffprocessor/resource_manager.go b/cmd/diff/diffprocessor/resource_manager.go index ea4577c3..8ef8f79f 100644 --- a/cmd/diff/diffprocessor/resource_manager.go +++ b/cmd/diff/diffprocessor/resource_manager.go @@ -157,7 +157,7 @@ func (m *DefaultResourceManager) checkCompositeOwnership(current *un.Unstructure } if labels := current.GetLabels(); labels != nil { - if owner, exists := labels["crossplane.io/composite"]; exists && owner != composite.GetName() { + if owner, exists := labels[LabelComposite]; exists && owner != composite.GetName() { // Log a warning if the resource is owned by a different composite m.logger.Info( // TODO: should we fail by default here? maybe require a --force flag to proceed? @@ -220,7 +220,7 @@ func (m *DefaultResourceManager) lookupByComposite(ctx context.Context, composit // If so, use that for lookup instead of the parent's name. This handles nested XRs // where the composed resources should have the ROOT XR's name in their composite label, // not the intermediate nested XR's name. - desiredCompositeLabel := desired.GetLabels()["crossplane.io/composite"] + desiredCompositeLabel := desired.GetLabels()[LabelComposite] switch { case isCompositeAClaim: @@ -229,8 +229,8 @@ func (m *DefaultResourceManager) lookupByComposite(ctx context.Context, composit // We'll use the claim labels to find downstream resources labelSelector = metav1.LabelSelector{ MatchLabels: map[string]string{ - "crossplane.io/claim-name": composite.GetName(), - "crossplane.io/claim-namespace": composite.GetNamespace(), + LabelClaimName: composite.GetName(), + LabelClaimNamespace: composite.GetNamespace(), }, } lookupName = composite.GetName() @@ -243,7 +243,7 @@ func (m *DefaultResourceManager) lookupByComposite(ctx context.Context, composit // Use the resource's own label value for lookup labelSelector = metav1.LabelSelector{ MatchLabels: map[string]string{ - "crossplane.io/composite": desiredCompositeLabel, + LabelComposite: desiredCompositeLabel, }, } lookupName = desiredCompositeLabel @@ -254,7 +254,7 @@ func (m *DefaultResourceManager) lookupByComposite(ctx context.Context, composit // For XRs, use the composite label labelSelector = metav1.LabelSelector{ MatchLabels: map[string]string{ - "crossplane.io/composite": composite.GetName(), + LabelComposite: composite.GetName(), }, } lookupName = composite.GetName() @@ -552,15 +552,15 @@ func (m *DefaultResourceManager) updateCompositeOwnerLabel(ctx context.Context, switch { case isParentAClaim: // For claims, set claim-specific labels (all claims are namespaced) - labels["crossplane.io/claim-name"] = parentName - labels["crossplane.io/claim-namespace"] = parent.GetNamespace() + labels[LabelClaimName] = parentName + labels[LabelClaimNamespace] = parent.GetNamespace() // For claims, only set the composite label if it doesn't already exist // If it exists, it likely points to a generated XR name which we should preserve - existingComposite, hasComposite := labels["crossplane.io/composite"] + existingComposite, hasComposite := labels[LabelComposite] if !hasComposite { // No existing composite label, set it to the claim name - labels["crossplane.io/composite"] = parentName + labels[LabelComposite] = parentName m.logger.Debug("Set composite label for new claim resource", "claimName", parentName, "claimNamespace", parent.GetNamespace(), @@ -576,10 +576,10 @@ func (m *DefaultResourceManager) updateCompositeOwnerLabel(ctx context.Context, default: // For XRs, only set the composite label if it doesn't already exist // If it exists, preserve it (e.g., for managed resources that already have correct ownership) - existingComposite, hasComposite := labels["crossplane.io/composite"] + existingComposite, hasComposite := labels[LabelComposite] if !hasComposite { // No existing composite label, set it to the parent XR name - labels["crossplane.io/composite"] = parentName + labels[LabelComposite] = parentName m.logger.Debug("Set composite label for new XR resource", "xrName", parentName, "child", child.GetName())