Skip to content

Conversation

@devivasudevan
Copy link
Contributor

@devivasudevan devivasudevan commented Dec 5, 2025

Signed-off-by: Devi Vasudevan [email protected]

What this PR does / why we need it:

Added kube-api-linter to linters and enabled the linters statusoptional and statussubresource - 4219

This PR is a feat.
Adds docker-based custom golangci-lint build

Fixes # 4219 #4219

Copilot AI review requested due to automatic review settings December 5, 2025 00:02
@devivasudevan devivasudevan requested a review from a team as a code owner December 5, 2025 00:02
@devivasudevan devivasudevan marked this pull request as draft December 5, 2025 00:03
@devivasudevan devivasudevan marked this pull request as ready for review December 5, 2025 00:04
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR integrates kube-api-linter into the project to enforce Kubernetes API conventions, specifically enabling the statusoptional and statussubresource linters. The changes add +optional markers to status fields across multiple API types and configure the linting infrastructure.

Key changes:

  • Added kube-api-linter configuration with custom plugin setup
  • Annotated status struct fields with +optional markers across all API versions
  • Added +kubebuilder:subresource:status markers to ProviderPodStatus and ConnectionPodStatus types
  • Modified the Makefile lint target to use the custom linter

Reviewed changes

Copilot reviewed 29 out of 29 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
.custom-gcl.yaml New configuration file for golangci-lint custom plugin with kube-api-linter
.golangci.yaml Enabled kubeapilinter with statusoptional and statussubresource checks
Makefile Updated lint target to build and run custom golangci-lint with kube-api-linter
apis/status/v1beta1/providerpodstatus_types.go Added +optional markers to status fields and +kubebuilder:subresource:status
apis/status/v1beta1/mutatorpodstatus_types.go Added +optional markers to status fields
apis/status/v1beta1/expansiontemplatepodstatus_types.go Added +optional markers to status fields
apis/status/v1beta1/constrainttemplatepodstatus_types.go Added +optional markers to status fields
apis/status/v1beta1/constraintpodstatus_types.go Added +optional markers to status fields
apis/status/v1beta1/configpodstatus_types.go Added +optional markers to status fields
apis/status/v1alpha1/connectionpodstatus_types.go Added +optional markers to status fields and +kubebuilder:subresource:status
apis/mutations/v1beta1/*.go Added +optional marker to ByPod status field
apis/mutations/v1alpha1/*.go Added +optional marker to ByPod status field
apis/mutations/v1/*.go Added +optional marker to ByPod status field
apis/mutations/unversioned/*.go Added +optional marker to ByPod status field
apis/expansion/v1beta1/expansiontemplate_types.go Added +optional marker to ByPod status field
apis/expansion/v1alpha1/expansiontemplate_types.go Added +optional marker to ByPod status field
apis/expansion/unversioned/expansiontemplate_types.go Added +optional marker to ByPod status field
apis/connection/v1alpha1/connection_types.go Changed Driver JSON tag to include omitempty and added +optional to ByPod field
apis/config/v1alpha1/config_types.go Added +optional marker to ByPod status field

@devivasudevan devivasudevan changed the title add kube-api-linter to linters - 4219 feat: Added kube-api-linter to linters - 4219 Dec 5, 2025
@codecov-commenter
Copy link

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 40.66%. Comparing base (3350319) to head (c3e1af5).
⚠️ Report is 514 commits behind head on master.

❗ There is a different number of reports uploaded between BASE (3350319) and HEAD (c3e1af5). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (3350319) HEAD (c3e1af5)
unittests 2 1
Additional details and impacted files
@@             Coverage Diff             @@
##           master    #4284       +/-   ##
===========================================
- Coverage   54.49%   40.66%   -13.84%     
===========================================
  Files         134      251      +117     
  Lines       12329    17723     +5394     
===========================================
+ Hits         6719     7207      +488     
- Misses       5116     9889     +4773     
- Partials      494      627      +133     
Flag Coverage Δ
unittests 40.66% <ø> (-13.84%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot AI review requested due to automatic review settings December 5, 2025 13:46
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 34 out of 34 changed files in this pull request and generated 4 comments.

text: 'deprecated: This package is intended for older projects transitioning from OPA v0.x and will remain for the lifetime of OPA v1.x'
- linters:
- kubeapilinter
path-except: apis/*
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The path-except configuration appears to be inverted. Using path-except: apis/* will exclude the apis/ directory from the kubeapilinter checks, but the purpose of this PR is to lint the Kubernetes API types in the apis/ directory. This should likely be path: apis/* instead to include only the apis directory, or the exclusion rule should be removed entirely to lint all paths.

Suggested change
path-except: apis/*
path: apis/*

Copilot uses AI. Check for mistakes.
-v ${GOLANGCI_LINT_CACHE}:/root/.cache/golangci-lint \
-w /app golangci/golangci-lint:${GOLANGCI_LINT_VERSION} \
sh -c "test -f /app/bin/golangci-lint-kube-api-linter || golangci-lint custom ; \
/app/bin/golangci-lint-kube-api-linter run --timeout=$(TIMEOUT) --fix --concurrency 2 "
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The shell command has a trailing space after --concurrency 2 which could cause issues. The space before the closing quote should be removed: change --concurrency 2 " to --concurrency 2".

Suggested change
/app/bin/golangci-lint-kube-api-linter run --timeout=$(TIMEOUT) --fix --concurrency 2 "
/app/bin/golangci-lint-kube-api-linter run --timeout=$(TIMEOUT) --fix --concurrency 2"

Copilot uses AI. Check for mistakes.
Comment on lines +384 to +386

# Run using Docker

Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The empty line comment # Run using Docker should be placed before the lint: target declaration, not after it. Currently, the target has no description on the same line or immediately before it, which could confuse readers about what follows. Move line 385-386 to lines 383-384 for better readability.

Copilot uses AI. Check for mistakes.
@@ -0,0 +1,7 @@
version: v2.6.2
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Version mismatch between Makefile and .custom-gcl.yaml: The Makefile specifies GOLANGCI_LINT_VERSION := v2.4.0 while .custom-gcl.yaml uses version: v2.6.2. This inconsistency could lead to unexpected behavior or build failures. These versions should be synchronized to ensure consistent linting behavior across the build process.

Suggested change
version: v2.6.2
version: v2.4.0

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants