Add API param linter (#44045)
Adds a linter to ensure we don't add new instances of `team` or `query`
in API params. This will be used incrementally, but this PR also adds
`nolint` directives to places that still have these terms, both to avoid
false-positives later and to help with full migration away from these
terms in in Fleet 5.
Example:
```
server/fleet/campaigns.go:51:16: json tag "team_id": uses deprecated "team"/"teams" — use "fleet"/"fleets" instead (apiparamcheck)
Team *uint `json:"team_id,omitempty"`
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Added a new static analyzer (apiparamcheck) to flag deprecated API
parameter names ("team/teams") and improper usages of "query/queries".
* **Chores**
* Integrated the new check into CI tooling and configuration.
* Added analyzer tests and plugin registration.
* Applied targeted lint-suppression annotations across code and tests
where legacy parameter names must remain.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---------
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This commit is contained in:
co-authored by
Copilot Autofix powered by AI
parent
0f94afe29b
commit
4daed869ad
@@ -9,3 +9,6 @@ plugins:
|
||||
- module: "github.com/fleetdm/fleet/v4/tools/ci/setboolcheck"
|
||||
import: "github.com/fleetdm/fleet/v4/tools/ci/setboolcheck/cmd/gclplugin"
|
||||
path: "tools/ci/setboolcheck"
|
||||
- module: "github.com/fleetdm/fleet/v4/tools/ci/apiparamcheck"
|
||||
import: "github.com/fleetdm/fleet/v4/tools/ci/apiparamcheck/cmd/gclplugin"
|
||||
path: "tools/ci/apiparamcheck"
|
||||
|
||||
@@ -16,6 +16,7 @@ linters:
|
||||
- nilaway
|
||||
- setboolcheck
|
||||
- depguard
|
||||
- apiparamcheck
|
||||
settings:
|
||||
gosec:
|
||||
# Only enable rules that are too noisy on existing code but valuable for new code.
|
||||
@@ -50,6 +51,9 @@ linters:
|
||||
setboolcheck:
|
||||
type: module
|
||||
description: Flags map[T]bool used as sets; suggests map[T]struct{} instead.
|
||||
apiparamcheck:
|
||||
type: module
|
||||
description: Flags json/url/query struct tags using deprecated "team"/"teams" and "query"/"queries" terms.
|
||||
exclusions:
|
||||
generated: strict
|
||||
rules:
|
||||
|
||||
@@ -900,7 +900,7 @@ func (cmd *GenerateGitopsCommand) generateSSOSettings(ssoSettings *fleet.SSOSett
|
||||
|
||||
type GlobalOrTeamIntegrations struct {
|
||||
GlobalIntegrations *fleet.Integrations `json:"global_integrations,omitempty"`
|
||||
TeamIntegrations *fleet.TeamIntegrations `json:"team_integrations,omitempty"`
|
||||
TeamIntegrations *fleet.TeamIntegrations `json:"team_integrations,omitempty"` //nolint:apiparamcheck // internal routing key, not emitted to user
|
||||
}
|
||||
|
||||
func (cmd *GenerateGitopsCommand) generateIntegrations(filePath string, integrations *GlobalOrTeamIntegrations) (map[string]interface{}, error) {
|
||||
|
||||
@@ -229,13 +229,13 @@ type UserRoles struct {
|
||||
}
|
||||
|
||||
type TeamRole struct {
|
||||
Team string `json:"team"`
|
||||
Team string `json:"team" renameto:"fleet"` // renameto doesn't actually do anything here but adding for visibility
|
||||
Role string `json:"role"`
|
||||
}
|
||||
|
||||
type UserRole struct {
|
||||
GlobalRole *string `json:"global_role"`
|
||||
Teams []TeamRole `json:"teams"`
|
||||
Teams []TeamRole `json:"teams" renameto:"fleets"` // renameto doesn't actually do anything here but adding for visibility
|
||||
}
|
||||
|
||||
func usersToUserRoles(users []fleet.User) UserRoles {
|
||||
|
||||
@@ -48,13 +48,13 @@ type testAppConfig struct {
|
||||
} `json:"host_expiry_settings"`
|
||||
ServerSettings struct {
|
||||
ServerURL string `json:"server_url"`
|
||||
LiveQueryDisabled bool `json:"live_query_disabled"`
|
||||
LiveQueryDisabled bool `json:"live_query_disabled"` //nolint:apiparamcheck // frozen migration; preserves historical shape
|
||||
EnableAnalytics bool `json:"enable_analytics"`
|
||||
} `json:"server_settings"`
|
||||
HostSettings struct {
|
||||
EnableHostUsers bool `json:"enable_host_users"`
|
||||
EnableSoftwareInventory bool `json:"enable_software_inventory"`
|
||||
AdditionalQueries *json.RawMessage `json:"additional_queries,omitempty"`
|
||||
AdditionalQueries *json.RawMessage `json:"additional_queries,omitempty"` //nolint:apiparamcheck // frozen migration; preserves historical shape
|
||||
} `json:"host_settings"`
|
||||
VulnerabilitySettings struct {
|
||||
DatabasesPath string `json:"databases_path"`
|
||||
|
||||
+1
-1
@@ -31,7 +31,7 @@ func Up_20240320145650(tx *sql.Tx) error {
|
||||
|
||||
type macosSetupAssistantArgs struct {
|
||||
Task string `json:"task"`
|
||||
TeamID *uint `json:"team_id,omitempty"`
|
||||
TeamID *uint `json:"team_id,omitempty"` //nolint:apiparamcheck // frozen migration; preserves historical shape
|
||||
HostSerialNumbers []string `json:"host_serial_numbers,omitempty"`
|
||||
}
|
||||
argsJSON, err := json.Marshal(macosSetupAssistantArgs{Task: taskName})
|
||||
|
||||
+1
-1
@@ -13,7 +13,7 @@ func TestUp_20240320145650(t *testing.T) {
|
||||
|
||||
type macosSetupAssistantArgs struct {
|
||||
Task string `json:"task"`
|
||||
TeamID *uint `json:"team_id,omitempty"`
|
||||
TeamID *uint `json:"team_id,omitempty"` //nolint:apiparamcheck // matches frozen migration payload shape
|
||||
HostSerialNumbers []string `json:"host_serial_numbers,omitempty"`
|
||||
}
|
||||
|
||||
|
||||
@@ -205,7 +205,7 @@ type osqueryAgentOptions struct {
|
||||
FileAccesses []string `json:"file_accesses"`
|
||||
// Documentation for the following 2 fields is "hidden" in osquery's FIM page:
|
||||
// https://osquery.readthedocs.io/en/stable/deployment/file-integrity-monitoring/
|
||||
FilePathsQuery map[string][]string `json:"file_paths_query"`
|
||||
FilePathsQuery map[string][]string `json:"file_paths_query"` //nolint:apiparamcheck // osquery FIM field
|
||||
ExcludePaths map[string][]string `json:"exclude_paths"`
|
||||
|
||||
YARA struct {
|
||||
|
||||
+3
-3
@@ -189,7 +189,7 @@ type MDM struct {
|
||||
AppleServerURL string `json:"apple_server_url"`
|
||||
|
||||
// Deprecated: use AppleBussinessManager instead
|
||||
DeprecatedAppleBMDefaultTeam string `json:"apple_bm_default_team,omitempty"`
|
||||
DeprecatedAppleBMDefaultTeam string `json:"apple_bm_default_team,omitempty"` //nolint:apiparamcheck // not renaming already-deprecated field
|
||||
|
||||
// AppleBusinessManager defines the associations between ABM tokens
|
||||
// and the teams used to assign hosts when they're ingested from ABM.
|
||||
@@ -1348,8 +1348,8 @@ type ActivityExpirySettings struct {
|
||||
type Features struct {
|
||||
EnableHostUsers bool `json:"enable_host_users"`
|
||||
EnableSoftwareInventory bool `json:"enable_software_inventory"`
|
||||
AdditionalQueries *json.RawMessage `json:"additional_queries,omitempty"`
|
||||
DetailQueryOverrides map[string]*string `json:"detail_query_overrides,omitempty"`
|
||||
AdditionalQueries *json.RawMessage `json:"additional_queries,omitempty"` //nolint:apiparamcheck // osquery host-details queries
|
||||
DetailQueryOverrides map[string]*string `json:"detail_query_overrides,omitempty"` //nolint:apiparamcheck // osquery detail-query overrides
|
||||
HistoricalData HistoricalDataSettings `json:"historical_data"`
|
||||
|
||||
/////////////////////////////////////////////////////////////////
|
||||
|
||||
@@ -39,7 +39,7 @@ type DistributedQueryCampaignTarget struct {
|
||||
// thus it should be kept as small as possible.
|
||||
type DistributedQueryResult struct {
|
||||
// DistributedQueryCampaignID is the unique ID of the live query campaign.
|
||||
DistributedQueryCampaignID uint `json:"distributed_query_execution_id"`
|
||||
DistributedQueryCampaignID uint `json:"distributed_query_execution_id"` //nolint:apiparamcheck // osquery distributed-query concept
|
||||
// Host holds the host's data from where the query result comes from.
|
||||
Host ResultHostData `json:"host"`
|
||||
Rows []map[string]string `json:"rows"`
|
||||
|
||||
+2
-2
@@ -7,10 +7,10 @@ import "github.com/jmoiron/sqlx"
|
||||
type DBLock struct {
|
||||
WaitingTrxID string `db:"waiting_trx_id" json:"waiting_trx_id"`
|
||||
WaitingThread uint64 `db:"waiting_thread" json:"waiting_thread"`
|
||||
WaitingQuery *string `db:"waiting_query" json:"waiting_query,omitempty"`
|
||||
WaitingQuery *string `db:"waiting_query" json:"waiting_query,omitempty"` //nolint:apiparamcheck // MySQL InnoDB lock query text
|
||||
BlockingTrxID string `db:"blocking_trx_id" json:"blocking_trx_id"`
|
||||
BlockingThread uint64 `db:"blocking_thread" json:"blocking_thread"`
|
||||
BlockingQuery *string `db:"blocking_query" json:"blocking_query,omitempty"`
|
||||
BlockingQuery *string `db:"blocking_query" json:"blocking_query,omitempty"` //nolint:apiparamcheck // MySQL InnoDB lock query text
|
||||
}
|
||||
|
||||
// DBReader is an interface that defines the methods required for reads.
|
||||
|
||||
@@ -395,7 +395,7 @@ type Host struct {
|
||||
// add a "reason" field with well-known labels so we know what condition(s)
|
||||
// are expected to clear the timestamp. For now there's a single use-case
|
||||
// so we don't need this.
|
||||
RefetchCriticalQueriesUntil *time.Time `json:"refetch_critical_queries_until" db:"refetch_critical_queries_until" csv:"-"`
|
||||
RefetchCriticalQueriesUntil *time.Time `json:"refetch_critical_queries_until" db:"refetch_critical_queries_until" csv:"-"` //nolint:apiparamcheck
|
||||
|
||||
// DEPAssignedToFleet is set to true if the host is assigned to Fleet in Apple Business.
|
||||
// It is a *bool becase we want it to be returned from only a subset of endpoints related to
|
||||
|
||||
@@ -600,7 +600,7 @@ type SoftwareInstallerPayload struct {
|
||||
// the path field, this uses "script://filename" to pass the filename; in that
|
||||
// case InstallScript contains the script content directly.
|
||||
URL string `json:"url"`
|
||||
PreInstallQuery string `json:"pre_install_query"`
|
||||
PreInstallQuery string `json:"pre_install_query"` //nolint:apiparamcheck // SQL precondition for install
|
||||
// InstallScript is the script to run after downloading the installer. For script
|
||||
// packages via "script://" URL, this contains the package content itself.
|
||||
InstallScript string `json:"install_script"`
|
||||
|
||||
@@ -629,7 +629,7 @@ func (hse *HostSoftwareEntry) UnmarshalJSON(b []byte) error {
|
||||
|
||||
type PathSignatureInformation struct {
|
||||
InstalledPath string `json:"installed_path"`
|
||||
TeamIdentifier string `json:"team_identifier"`
|
||||
TeamIdentifier string `json:"team_identifier"` //nolint:apiparamcheck // Apple developer team identifier (code-signing)
|
||||
// json struct tag difference here is for backwards compatibility. API field was initially "hash_sha256", though it is specifically the CD hash (sha256).
|
||||
CDHashSHA256 *string `json:"hash_sha256"`
|
||||
ExecutableSHA256 *string `json:"executable_sha256"`
|
||||
|
||||
@@ -95,7 +95,7 @@ type SoftwareInstaller struct {
|
||||
// UninstallScriptContentID is the ID of the uninstall script content.
|
||||
UninstallScriptContentID uint `json:"-" db:"uninstall_script_content_id"`
|
||||
// PreInstallQuery is the query to run as a condition to installing the software package.
|
||||
PreInstallQuery string `json:"pre_install_query" db:"pre_install_query"`
|
||||
PreInstallQuery string `json:"pre_install_query" db:"pre_install_query"` //nolint:apiparamcheck // SQL precondition for install
|
||||
// PostInstallScript is the script to run after installing the software package.
|
||||
PostInstallScript string `json:"post_install_script" db:"post_install_script"`
|
||||
// UninstallScript is the script to run to uninstall the software package.
|
||||
@@ -423,7 +423,7 @@ type HostSoftwareInstallerResult struct {
|
||||
// Output is the output of the software installer package on the host.
|
||||
Output *string `json:"output" db:"install_script_output"`
|
||||
// PreInstallQueryOutput is the output of the pre-install query on the host.
|
||||
PreInstallQueryOutput *string `json:"pre_install_query_output" db:"pre_install_query_output"`
|
||||
PreInstallQueryOutput *string `json:"pre_install_query_output" db:"pre_install_query_output"` //nolint:apiparamcheck // SQL precondition output
|
||||
// PostInstallScriptOutput is the output of the post-install script on the host.
|
||||
PostInstallScriptOutput *string `json:"post_install_script_output" db:"post_install_script_output"`
|
||||
// CreatedAt is the time the software installer request was triggered.
|
||||
@@ -825,7 +825,7 @@ func (s *SoftwarePackageOrApp) FullyQualifiedName() string {
|
||||
type SoftwarePackageSpec struct {
|
||||
URL string `json:"url"`
|
||||
SelfService bool `json:"self_service"`
|
||||
PreInstallQuery TeamSpecSoftwareAsset `json:"pre_install_query"`
|
||||
PreInstallQuery TeamSpecSoftwareAsset `json:"pre_install_query"` //nolint:apiparamcheck // SQL precondition for install
|
||||
InstallScript TeamSpecSoftwareAsset `json:"install_script"`
|
||||
PostInstallScript TeamSpecSoftwareAsset `json:"post_install_script"`
|
||||
UninstallScript TeamSpecSoftwareAsset `json:"uninstall_script"`
|
||||
@@ -885,7 +885,7 @@ type MaintainedAppSpec struct {
|
||||
Slug string `json:"slug"`
|
||||
Version string `json:"version"`
|
||||
SelfService bool `json:"self_service"`
|
||||
PreInstallQuery TeamSpecSoftwareAsset `json:"pre_install_query"`
|
||||
PreInstallQuery TeamSpecSoftwareAsset `json:"pre_install_query"` //nolint:apiparamcheck // SQL precondition for install
|
||||
InstallScript TeamSpecSoftwareAsset `json:"install_script"`
|
||||
PostInstallScript TeamSpecSoftwareAsset `json:"post_install_script"`
|
||||
UninstallScript TeamSpecSoftwareAsset `json:"uninstall_script"`
|
||||
|
||||
@@ -26,7 +26,7 @@ type SoftwareTitleIcon struct {
|
||||
// can properly check team ownership (rego marshals the struct to JSON to pass it to
|
||||
// the rego policies script). This struct is never marshalled directly to JSON in
|
||||
// API responses at this time so it doesn't affect anything else.
|
||||
TeamID uint `db:"team_id" json:"team_id"` // TODO -- rename to `fleet_id` when authz code switches to using `fleet_id` instead of `team_id`
|
||||
TeamID uint `db:"team_id" json:"team_id"` //nolint:apiparamcheck // TODO -- rename when authz code switches to using `fleet_id` instead of `team_id`
|
||||
SoftwareTitleID uint `db:"software_title_id"`
|
||||
StorageID string `db:"storage_id"`
|
||||
Filename string `db:"filename"`
|
||||
|
||||
@@ -19,9 +19,9 @@ type StatisticsPayload struct {
|
||||
NumHostSoftwareInstalledPaths int `json:"numHostSoftwareInstalledPaths"`
|
||||
NumSoftwareCPEs int `json:"numSoftwareCPEs"`
|
||||
NumSoftwareCVEs int `json:"numSoftwareCVEs"`
|
||||
NumTeams int `json:"numTeams"`
|
||||
NumTeams int `json:"numTeams"` //nolint:apiparamcheck // don't want to break analytics ingestion
|
||||
NumPolicies int `json:"numPolicies"`
|
||||
NumQueries int `json:"numQueries"`
|
||||
NumQueries int `json:"numQueries"` //nolint:apiparamcheck // don't want to break analytics ingestion
|
||||
NumLabels int `json:"numLabels"`
|
||||
SoftwareInventoryEnabled bool `json:"softwareInventoryEnabled"`
|
||||
VulnDetectionEnabled bool `json:"vulnDetectionEnabled"`
|
||||
@@ -31,7 +31,7 @@ type StatisticsPayload struct {
|
||||
HostExpiryEnabled bool `json:"hostExpiryEnabled"`
|
||||
MDMWindowsEnabled bool `json:"mdmWindowsEnabled"`
|
||||
MDMRecoveryLockPasswordEnabled bool `json:"mdmRecoveryLockPasswordEnabled"`
|
||||
LiveQueryDisabled bool `json:"liveQueryDisabled"`
|
||||
LiveQueryDisabled bool `json:"liveQueryDisabled"` //nolint:apiparamcheck // osquery live-query feature
|
||||
NumWeeklyActiveUsers int `json:"numWeeklyActiveUsers"`
|
||||
// NumWeeklyPolicyViolationDaysActual is an aggregate count of actual policy violation days. One
|
||||
// policy violation day is added for each policy that a host is failing as of the time the count
|
||||
|
||||
@@ -407,7 +407,7 @@ func TestDuplicateJSONKeysWithEncoder(t *testing.T) {
|
||||
}
|
||||
|
||||
type response struct {
|
||||
TeamID int `json:"team_id"`
|
||||
TeamID int `json:"team_id"` //nolint:apiparamcheck // rename handled centrally by spec.DeprecatedGitOpsKeyMappings
|
||||
Name string `json:"name"`
|
||||
}
|
||||
|
||||
|
||||
@@ -336,7 +336,7 @@ func TestJSONKeyRewriteReader_WithJSONDecoderOldKey(t *testing.T) {
|
||||
rewriter := NewJSONKeyRewriteReader(strings.NewReader(input), rules)
|
||||
|
||||
type request struct {
|
||||
TeamID int `json:"team_id"`
|
||||
TeamID int `json:"team_id"` //nolint:apiparamcheck // rename handled centrally by spec.DeprecatedGitOpsKeyMappings
|
||||
Name string `json:"name"`
|
||||
}
|
||||
var req request
|
||||
@@ -356,7 +356,7 @@ func TestJSONKeyRewriteReader_WithJSONDecoderNewKey(t *testing.T) {
|
||||
rewriter := NewJSONKeyRewriteReader(strings.NewReader(input), rules)
|
||||
|
||||
type request struct {
|
||||
TeamID int `json:"team_id"`
|
||||
TeamID int `json:"team_id"` //nolint:apiparamcheck // rename handled centrally by spec.DeprecatedGitOpsKeyMappings
|
||||
Name string `json:"name"`
|
||||
}
|
||||
var req request
|
||||
@@ -374,7 +374,7 @@ func TestJSONKeyRewriteReader_AliasConflictWithJSONDecoder(t *testing.T) {
|
||||
rewriter := NewJSONKeyRewriteReader(strings.NewReader(input), rules)
|
||||
|
||||
type request struct {
|
||||
TeamID int `json:"team_id"`
|
||||
TeamID int `json:"team_id"` //nolint:apiparamcheck // rename handled centrally by spec.DeprecatedGitOpsKeyMappings
|
||||
}
|
||||
var req request
|
||||
err := json.NewDecoder(rewriter).Decode(&req)
|
||||
|
||||
@@ -3008,7 +3008,7 @@ func (s *integrationEnterpriseTestSuite) TestNoTeamWebhookConfig() {
|
||||
// Test that we can configure webhooks for "No Team" (team ID 0)
|
||||
// Use a generic response that will work with DefaultTeam
|
||||
var defaultTeamResp struct {
|
||||
Team *fleet.DefaultTeam `json:"team"`
|
||||
Team *fleet.DefaultTeam `json:"team"` //nolint:apiparamcheck // test helper; matches server response shape
|
||||
}
|
||||
|
||||
// First clear any existing webhook configuration for "No Team"
|
||||
@@ -3042,7 +3042,7 @@ func (s *integrationEnterpriseTestSuite) TestNoTeamWebhookConfig() {
|
||||
|
||||
// Get the config again to verify it persisted
|
||||
defaultTeamResp = struct {
|
||||
Team *fleet.DefaultTeam `json:"team"`
|
||||
Team *fleet.DefaultTeam `json:"team"` //nolint:apiparamcheck // test helper; matches server response shape
|
||||
}{}
|
||||
s.DoJSON("GET", "/api/latest/fleet/teams/0", nil, http.StatusOK, &defaultTeamResp)
|
||||
require.Equal(t, uint(0), defaultTeamResp.Team.ID)
|
||||
@@ -3131,7 +3131,7 @@ func (s *integrationEnterpriseTestSuite) TestNoTeamFailingPolicyWebhookTrigger()
|
||||
|
||||
// Configure webhook for "No Team" - only include pol1 and pol2
|
||||
var defaultTeamResp struct {
|
||||
Team *fleet.DefaultTeam `json:"team"`
|
||||
Team *fleet.DefaultTeam `json:"team"` //nolint:apiparamcheck // test helper; matches server response shape
|
||||
}
|
||||
s.DoJSON("PATCH", "/api/latest/fleet/teams/0", fleet.TeamPayload{WebhookSettings: &fleet.TeamWebhookSettings{
|
||||
FailingPoliciesWebhook: fleet.FailingPoliciesWebhookSettings{
|
||||
@@ -3169,7 +3169,7 @@ func (s *integrationEnterpriseTestSuite) TestNoTeamFailingPolicyWebhookTrigger()
|
||||
|
||||
// Test that we can configure and retrieve the "No Team" webhook settings
|
||||
defaultTeamResp = struct {
|
||||
Team *fleet.DefaultTeam `json:"team"`
|
||||
Team *fleet.DefaultTeam `json:"team"` //nolint:apiparamcheck // test helper; matches server response shape
|
||||
}{}
|
||||
s.DoJSON("GET", "/api/latest/fleet/teams/0", nil, http.StatusOK, &defaultTeamResp)
|
||||
require.Equal(t, uint(0), defaultTeamResp.Team.ID)
|
||||
@@ -28834,7 +28834,7 @@ func (s *integrationEnterpriseTestSuite) TestCreateAPIOnlyUserPremium() {
|
||||
APIOnly bool `json:"api_only"`
|
||||
GlobalRole *string `json:"global_role"`
|
||||
APIEndpoints []apiEndpoint `json:"api_endpoints"`
|
||||
Teams []teamEntry `json:"teams"`
|
||||
Teams []teamEntry `json:"teams"` //nolint:apiparamcheck // test helper; matches server response shape
|
||||
} `json:"user"`
|
||||
Token string `json:"token"`
|
||||
Err string `json:"error,omitempty"`
|
||||
@@ -29009,7 +29009,7 @@ func (s *integrationEnterpriseTestSuite) TestModifyAPIOnlyUserPremium() {
|
||||
Teams []struct {
|
||||
ID uint `json:"id"`
|
||||
Role string `json:"role"`
|
||||
} `json:"teams"`
|
||||
} `json:"teams"` //nolint:apiparamcheck // test helper; matches server response shape
|
||||
} `json:"user"`
|
||||
Err string `json:"error,omitempty"`
|
||||
}
|
||||
|
||||
@@ -985,7 +985,7 @@ func (s *integrationMDMTestSuite) TestDEPProfileAssignment() {
|
||||
require.NotNil(t, pending[0].Args)
|
||||
var gotArgs struct {
|
||||
Task string `json:"task"`
|
||||
TeamID *uint `json:"team_id,omitempty"`
|
||||
TeamID *uint `json:"team_id,omitempty"` //nolint:apiparamcheck // matches worker job payload shape (see server/worker/macos_setup_assistant.go)
|
||||
HostSerialNumbers []string `json:"host_serial_numbers,omitempty"`
|
||||
}
|
||||
require.NoError(t, json.Unmarshal(*pending[0].Args, &gotArgs))
|
||||
|
||||
@@ -13,14 +13,14 @@ import (
|
||||
)
|
||||
|
||||
type addFleetMaintainedAppRequest struct {
|
||||
TeamID *uint `json:"team_id"`
|
||||
TeamID *uint `json:"team_id"` //nolint:apiparamcheck // alias handled manually via sibling FleetID field (see DecodeRequest below)
|
||||
// Note that we're adding an explicit FleetID field rather than using `renameto`.
|
||||
// The POST /software/fleet_maintained_apps endpoint has a custom decoder
|
||||
// and in this special case it's easier to handle the aliasing manually.
|
||||
FleetID *uint `json:"fleet_id"`
|
||||
AppID uint `json:"fleet_maintained_app_id"`
|
||||
InstallScript string `json:"install_script"`
|
||||
PreInstallQuery string `json:"pre_install_query"`
|
||||
PreInstallQuery string `json:"pre_install_query"` //nolint:apiparamcheck
|
||||
PostInstallScript string `json:"post_install_script"`
|
||||
SelfService bool `json:"self_service"`
|
||||
UninstallScript string `json:"uninstall_script"`
|
||||
|
||||
@@ -58,7 +58,7 @@ func (a *AppleMDM) Name() string {
|
||||
type appleMDMArgs struct {
|
||||
Task AppleMDMTask `json:"task"`
|
||||
HostUUID string `json:"host_uuid"`
|
||||
TeamID *uint `json:"team_id,omitempty"`
|
||||
TeamID *uint `json:"team_id,omitempty"` //nolint:apiparamcheck
|
||||
// EnrollReference is the UUID of the MDM IdP account used to enroll the
|
||||
// device. It is used to set the username and full name of the user
|
||||
// associated with the device.
|
||||
|
||||
@@ -46,7 +46,7 @@ func (m *MacosSetupAssistant) Name() string {
|
||||
// macosSetupAssistantArgs is the payload for the macos setup assistant job.
|
||||
type macosSetupAssistantArgs struct {
|
||||
Task MacosSetupAssistantTask `json:"task"`
|
||||
TeamID *uint `json:"team_id,omitempty"`
|
||||
TeamID *uint `json:"team_id,omitempty"` //nolint:apiparamcheck
|
||||
// Note that only DEP-enrolled hosts in Fleet MDM should be provided.
|
||||
HostSerialNumbers []string `json:"host_serial_numbers,omitempty"`
|
||||
}
|
||||
|
||||
@@ -46,13 +46,13 @@ type softwareWorkerArgs struct {
|
||||
EnterpriseName string `json:"enterprise_name,omitempty"`
|
||||
// AppTeamID is *not* a team ID, it is the vpp_apps_teams.id value. This is a bit confusing
|
||||
// as a name, but that is what is expected in this field.
|
||||
AppTeamID uint `json:"app_team_id,omitempty"`
|
||||
AppTeamID uint `json:"app_team_id,omitempty"` //nolint:apiparamcheck
|
||||
HostID uint `json:"host_id,omitempty"`
|
||||
|
||||
// HostEnrollTeamID is the team ID associated with the host at the time
|
||||
// of enrollment, which is the one used to run the setup experience.
|
||||
// A value of 0 is used to represent "no team".
|
||||
HostEnrollTeamID uint `json:"host_enroll_team_id,omitempty"`
|
||||
HostEnrollTeamID uint `json:"host_enroll_team_id,omitempty"` //nolint:apiparamcheck // not user-facing
|
||||
|
||||
// PolicyID is the Android Management API Policy ID associated with the host, *not*
|
||||
// a Fleet policy ID.
|
||||
@@ -281,7 +281,8 @@ func (v *SoftwareWorker) makeAndroidAppsAvailableForHost(ctx context.Context, ho
|
||||
}
|
||||
|
||||
func (v *SoftwareWorker) runAndroidSetupExperience(ctx context.Context,
|
||||
hostUUID string, hostEnrollTeamID uint, enterpriseName string) error {
|
||||
hostUUID string, hostEnrollTeamID uint, enterpriseName string,
|
||||
) error {
|
||||
host, err := v.Datastore.AndroidHostLiteByHostUUID(ctx, hostUUID)
|
||||
if err != nil {
|
||||
return ctxerr.Wrapf(ctx, err, "getting android host lite by uuid %s", hostUUID)
|
||||
@@ -400,8 +401,8 @@ func (v *SoftwareWorker) bulkMakeAndroidAppsAvailableForHost(ctx context.Context
|
||||
}
|
||||
|
||||
func buildApplicationPolicyWithConfig(ctx context.Context, appIDs []string,
|
||||
configsByAppID map[string]json.RawMessage, installType string) ([]*androidmanagement.ApplicationPolicy, error) {
|
||||
|
||||
configsByAppID map[string]json.RawMessage, installType string,
|
||||
) ([]*androidmanagement.ApplicationPolicy, error) {
|
||||
appPolicies := make([]*androidmanagement.ApplicationPolicy, 0, len(appIDs))
|
||||
for _, appID := range appIDs {
|
||||
var androidAppConfig struct {
|
||||
@@ -430,8 +431,8 @@ func buildApplicationPolicyWithConfig(ctx context.Context, appIDs []string,
|
||||
}
|
||||
|
||||
func QueueRunAndroidSetupExperience(ctx context.Context, ds fleet.Datastore, logger *slog.Logger,
|
||||
hostUUID string, hostEnrollTeamID *uint, enterpriseName string) error {
|
||||
|
||||
hostUUID string, hostEnrollTeamID *uint, enterpriseName string,
|
||||
) error {
|
||||
var enrollTeamID uint
|
||||
if hostEnrollTeamID != nil {
|
||||
enrollTeamID = *hostEnrollTeamID
|
||||
@@ -496,7 +497,6 @@ func QueueBulkSetAndroidAppsAvailableForHost(
|
||||
applicationIDs []string,
|
||||
enterpriseName string,
|
||||
) error {
|
||||
|
||||
args := &softwareWorkerArgs{
|
||||
Task: bulkSetAndroidAppsAvailableForHostTask,
|
||||
HostUUID: hostUUID,
|
||||
@@ -560,7 +560,6 @@ func (v *SoftwareWorker) bulkSetAndroidAppsAvailableForHosts(ctx context.Context
|
||||
}
|
||||
|
||||
err = v.AndroidModule.SetAppsForAndroidPolicy(ctx, enterpriseName, appPolicies, map[string]string{uuid: uuid})
|
||||
|
||||
if err != nil {
|
||||
return ctxerr.WrapWithData(ctx, err, "set apps for android policy", map[string]any{"host_id": hostID})
|
||||
}
|
||||
@@ -568,7 +567,6 @@ func (v *SoftwareWorker) bulkSetAndroidAppsAvailableForHosts(ctx context.Context
|
||||
}
|
||||
|
||||
return nil
|
||||
|
||||
}
|
||||
|
||||
func QueueBulkSetAndroidAppsAvailableForHosts(
|
||||
@@ -576,8 +574,8 @@ func QueueBulkSetAndroidAppsAvailableForHosts(
|
||||
ds fleet.Datastore,
|
||||
logger *slog.Logger,
|
||||
uuidsToIDs map[string]uint,
|
||||
enterpriseName string) error {
|
||||
|
||||
enterpriseName string,
|
||||
) error {
|
||||
args := &softwareWorkerArgs{
|
||||
Task: bulkSetAndroidAppsAvailableForHostsTask,
|
||||
UUIDsToIDs: uuidsToIDs,
|
||||
|
||||
@@ -44,7 +44,7 @@ type failingPolicyArgs struct {
|
||||
PolicyName string `json:"policy_name"`
|
||||
PolicyCritical bool `json:"policy_critical"`
|
||||
Hosts []fleet.PolicySetHost `json:"hosts"`
|
||||
TeamID *uint `json:"team_id,omitempty"`
|
||||
TeamID *uint `json:"team_id,omitempty"` //nolint:apiparamcheck // these are written to the db, changing likely requires migration
|
||||
}
|
||||
|
||||
// vulnArgs are the args common to all integrations that can process
|
||||
|
||||
@@ -0,0 +1,156 @@
|
||||
// Package apiparamcheck defines an analyzer that flags json/url/query struct
|
||||
// tags whose name contains deprecated Fleet terminology.
|
||||
//
|
||||
// Two renames are enforced:
|
||||
// - "team" / "teams" was renamed to "fleet" / "fleets". Any occurrence of
|
||||
// "team" or "teams" as a token in a tag name is flagged, including in
|
||||
// snake_case, camelCase/PascalCase, and kebab-case names.
|
||||
// - "query" / "queries" was renamed to "report" / "reports" when referring
|
||||
// to the product concept (the SQL sense of the word is fine). A tag name
|
||||
// of exactly "query" or "queries" is allowed; any larger name containing
|
||||
// "query" or "queries" as a token is flagged.
|
||||
package apiparamcheck
|
||||
|
||||
import (
|
||||
"go/ast"
|
||||
"reflect"
|
||||
"strconv"
|
||||
"strings"
|
||||
|
||||
"golang.org/x/tools/go/analysis"
|
||||
"golang.org/x/tools/go/analysis/passes/inspect"
|
||||
"golang.org/x/tools/go/ast/inspector"
|
||||
)
|
||||
|
||||
// Analyzer flags struct tags using deprecated "team"/"teams"/"query"/"queries"
|
||||
// terminology in json/url/query tag names.
|
||||
var Analyzer = &analysis.Analyzer{
|
||||
Name: "apiparamcheck",
|
||||
Doc: "flags json/url/query struct tags using deprecated team/teams or query/queries terms",
|
||||
URL: "https://github.com/fleetdm/fleet",
|
||||
Requires: []*analysis.Analyzer{inspect.Analyzer},
|
||||
Run: run,
|
||||
}
|
||||
|
||||
var checkedTagKeys = []string{"json", "url", "query"}
|
||||
|
||||
func run(pass *analysis.Pass) (any, error) {
|
||||
insp := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
|
||||
|
||||
insp.Preorder([]ast.Node{(*ast.StructType)(nil)}, func(n ast.Node) {
|
||||
st := n.(*ast.StructType)
|
||||
if st.Fields == nil {
|
||||
return
|
||||
}
|
||||
for _, field := range st.Fields.List {
|
||||
if field.Tag == nil {
|
||||
continue
|
||||
}
|
||||
unquoted, err := strconv.Unquote(field.Tag.Value)
|
||||
if err != nil {
|
||||
continue
|
||||
}
|
||||
tag := reflect.StructTag(unquoted)
|
||||
// A non-empty `renameto:"new_name"` tag opts the field out of this
|
||||
// check. It declares the new tag name this field will eventually
|
||||
// be renamed to, letting us grandfather legacy names while still
|
||||
// flagging anything new.
|
||||
if renameTo, ok := tag.Lookup("renameto"); ok && renameTo != "" {
|
||||
continue
|
||||
}
|
||||
for _, key := range checkedTagKeys {
|
||||
v, ok := tag.Lookup(key)
|
||||
if !ok {
|
||||
continue
|
||||
}
|
||||
name, _, _ := strings.Cut(v, ",")
|
||||
if msg := violationMessage(name); msg != "" {
|
||||
pass.Reportf(field.Tag.Pos(), "%s tag %q: %s", key, name, msg)
|
||||
}
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
// violationMessage returns a non-empty message describing why the tag name
|
||||
// is invalid, or "" if the name is allowed.
|
||||
func violationMessage(name string) string {
|
||||
tokens := splitTokens(name)
|
||||
if len(tokens) == 0 {
|
||||
return ""
|
||||
}
|
||||
for _, t := range tokens {
|
||||
low := strings.ToLower(t)
|
||||
if low == "team" || low == "teams" {
|
||||
return `uses deprecated "team"/"teams" — use "fleet"/"fleets" instead`
|
||||
}
|
||||
}
|
||||
// "query"/"queries" are allowed when they are the entire tag name
|
||||
// (referring to a SQL query), but not as part of a larger name.
|
||||
if len(tokens) > 1 {
|
||||
for _, t := range tokens {
|
||||
low := strings.ToLower(t)
|
||||
if low == "query" || low == "queries" {
|
||||
return `uses "query"/"queries" as part of a name — use "report"/"reports" instead (bare "query"/"queries" is ok)`
|
||||
}
|
||||
}
|
||||
}
|
||||
return ""
|
||||
}
|
||||
|
||||
// splitTokens splits a tag name into its snake_case and camelCase components.
|
||||
// Examples:
|
||||
//
|
||||
// "team_id" -> ["team", "id"]
|
||||
// "hostTeamID" -> ["host", "Team", "ID"]
|
||||
// "query" -> ["query"]
|
||||
// "osquery_version" -> ["osquery", "version"]
|
||||
func splitTokens(name string) []string {
|
||||
if name == "" {
|
||||
return nil
|
||||
}
|
||||
// First split on underscores and hyphens (snake_case / kebab-case).
|
||||
var out []string
|
||||
for _, part := range strings.FieldsFunc(name, func(r rune) bool {
|
||||
return r == '_' || r == '-'
|
||||
}) {
|
||||
out = append(out, splitCamel(part)...)
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// splitCamel splits a camelCase or PascalCase identifier into its components.
|
||||
// Runs of consecutive uppercase letters are kept together (e.g. "ID").
|
||||
func splitCamel(s string) []string {
|
||||
if s == "" {
|
||||
return nil
|
||||
}
|
||||
runes := []rune(s)
|
||||
var parts []string
|
||||
start := 0
|
||||
for i := 1; i < len(runes); i++ {
|
||||
cur, prev := runes[i], runes[i-1]
|
||||
// Boundary: lowercase/digit followed by uppercase.
|
||||
if isUpper(cur) && !isUpper(prev) {
|
||||
parts = append(parts, string(runes[start:i]))
|
||||
start = i
|
||||
continue
|
||||
}
|
||||
// Boundary: end of an uppercase run before a lowercase
|
||||
// (e.g. "IDs" -> "I", "Ds" is wrong; "IDs" should split as
|
||||
// "ID"+"s" — detect the split one rune earlier).
|
||||
if i+1 < len(runes) && isUpper(prev) && isUpper(cur) && !isUpper(runes[i+1]) {
|
||||
parts = append(parts, string(runes[start:i]))
|
||||
start = i
|
||||
continue
|
||||
}
|
||||
}
|
||||
parts = append(parts, string(runes[start:]))
|
||||
return parts
|
||||
}
|
||||
|
||||
func isUpper(r rune) bool {
|
||||
return r >= 'A' && r <= 'Z'
|
||||
}
|
||||
@@ -0,0 +1,13 @@
|
||||
package apiparamcheck_test
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/fleetdm/fleet/v4/tools/ci/apiparamcheck"
|
||||
"golang.org/x/tools/go/analysis/analysistest"
|
||||
)
|
||||
|
||||
func TestAnalyzer(t *testing.T) {
|
||||
testdata := analysistest.TestData()
|
||||
analysistest.Run(t, testdata, apiparamcheck.Analyzer, "example")
|
||||
}
|
||||
@@ -0,0 +1,27 @@
|
||||
// Package gclplugin provides the golangci-lint module plugin entry point.
|
||||
package gclplugin
|
||||
|
||||
import (
|
||||
"github.com/fleetdm/fleet/v4/tools/ci/apiparamcheck"
|
||||
"github.com/golangci/plugin-module-register/register"
|
||||
"golang.org/x/tools/go/analysis"
|
||||
)
|
||||
|
||||
func init() {
|
||||
register.Plugin("apiparamcheck", New)
|
||||
}
|
||||
|
||||
// New returns the golangci-lint plugin for the apiparamcheck analyzer.
|
||||
func New(_ any) (register.LinterPlugin, error) {
|
||||
return &plugin{}, nil
|
||||
}
|
||||
|
||||
type plugin struct{}
|
||||
|
||||
func (p *plugin) BuildAnalyzers() ([]*analysis.Analyzer, error) {
|
||||
return []*analysis.Analyzer{apiparamcheck.Analyzer}, nil
|
||||
}
|
||||
|
||||
func (p *plugin) GetLoadMode() string {
|
||||
return register.LoadModeSyntax
|
||||
}
|
||||
@@ -0,0 +1,13 @@
|
||||
module github.com/fleetdm/fleet/v4/tools/ci/apiparamcheck
|
||||
|
||||
go 1.26.2
|
||||
|
||||
require (
|
||||
github.com/golangci/plugin-module-register v0.1.2
|
||||
golang.org/x/tools v0.42.0
|
||||
)
|
||||
|
||||
require (
|
||||
golang.org/x/mod v0.33.0 // indirect
|
||||
golang.org/x/sync v0.19.0 // indirect
|
||||
)
|
||||
@@ -0,0 +1,10 @@
|
||||
github.com/golangci/plugin-module-register v0.1.2 h1:e5WM6PO6NIAEcij3B053CohVp3HIYbzSuP53UAYgOpg=
|
||||
github.com/golangci/plugin-module-register v0.1.2/go.mod h1:1+QGTsKBvAIvPvoY/os+G5eoqxWn70HYDm2uvUyGuVw=
|
||||
github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
|
||||
github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
|
||||
golang.org/x/mod v0.33.0 h1:tHFzIWbBifEmbwtGz65eaWyGiGZatSrT9prnU8DbVL8=
|
||||
golang.org/x/mod v0.33.0/go.mod h1:swjeQEj+6r7fODbD2cqrnje9PnziFuw4bmLbBZFrQ5w=
|
||||
golang.org/x/sync v0.19.0 h1:vV+1eWNmZ5geRlYjzm2adRgW2/mcpevXNg50YZtPCE4=
|
||||
golang.org/x/sync v0.19.0/go.mod h1:9KTHXmSnoGruLpwFjVSX0lNNA75CykiMECbovNTZqGI=
|
||||
golang.org/x/tools v0.42.0 h1:uNgphsn75Tdz5Ji2q36v/nsFSfR/9BRFvqhGBaJGd5k=
|
||||
golang.org/x/tools v0.42.0/go.mod h1:Ma6lCIwGZvHK6XtgbswSoWroEkhugApmsXyrUmBhfr0=
|
||||
@@ -0,0 +1,49 @@
|
||||
package example
|
||||
|
||||
// --- team / teams should be flagged ---
|
||||
|
||||
type teamsBad struct {
|
||||
A string `json:"team"` // want `json tag "team": uses deprecated "team"/"teams"`
|
||||
B string `json:"teams"` // want `json tag "teams": uses deprecated "team"/"teams"`
|
||||
C string `json:"team_id"` // want `json tag "team_id": uses deprecated "team"/"teams"`
|
||||
D string `url:"host_team_id"` // want `url tag "host_team_id": uses deprecated "team"/"teams"`
|
||||
E string `query:"my_teams"` // want `query tag "my_teams": uses deprecated "team"/"teams"`
|
||||
F string `json:"numTeams"` // want `json tag "numTeams": uses deprecated "team"/"teams"`
|
||||
G string `json:"team,omitempty"` // want `json tag "team": uses deprecated "team"/"teams"`
|
||||
}
|
||||
|
||||
// --- query / queries as part of a larger name should be flagged ---
|
||||
|
||||
type queriesBad struct {
|
||||
H string `json:"query_id"` // want `json tag "query_id": uses "query"/"queries" as part of a name`
|
||||
I string `json:"saved_queries"` // want `json tag "saved_queries": uses "query"/"queries" as part of a name`
|
||||
J string `url:"my_query"` // want `url tag "my_query": uses "query"/"queries" as part of a name`
|
||||
K string `json:"queryName"` // want `json tag "queryName": uses "query"/"queries" as part of a name`
|
||||
L string `json:"scheduled_query_id"` // want `json tag "scheduled_query_id": uses "query"/"queries" as part of a name`
|
||||
}
|
||||
|
||||
// --- renameto escape hatch ---
|
||||
|
||||
type renameToEscape struct {
|
||||
// Non-empty renameto suppresses the check.
|
||||
AA string `json:"team_id" renameto:"fleet_id"`
|
||||
BB string `json:"query_id" renameto:"report_id"`
|
||||
CC string `url:"teams" renameto:"fleets"`
|
||||
// Empty renameto does NOT suppress.
|
||||
DD string `json:"team_id" renameto:""` // want `json tag "team_id": uses deprecated "team"/"teams"`
|
||||
}
|
||||
|
||||
// --- allowed ---
|
||||
|
||||
type okStruct struct {
|
||||
M string `json:"fleet_id"`
|
||||
N string `json:"report_id"`
|
||||
O string `json:"query"`
|
||||
P string `json:"queries"`
|
||||
Q string `json:"osquery_version"`
|
||||
R string `json:"osquery"`
|
||||
S string `json:"stream_name"`
|
||||
T string `json:"query,omitempty"`
|
||||
U string `db:"team_id"` // db tag is not checked
|
||||
V string `json:"-"`
|
||||
}
|
||||
Reference in New Issue
Block a user