Skip to content

feat: add license resource type for Snowflake edition data#129

Open
c1-dev-bot[bot] wants to merge 1 commit into
mainfrom
cxh-1935/add-license-data
Open

feat: add license resource type for Snowflake edition data#129
c1-dev-bot[bot] wants to merge 1 commit into
mainfrom
cxh-1935/add-license-data

Conversation

@c1-dev-bot

@c1-dev-bot c1-dev-bot Bot commented Jun 29, 2026

Copy link
Copy Markdown

Summary

Adds a license resource type to the Snowflake connector that exposes account edition information as a license resource with TRAIT_LICENSE_PROFILE.

  • Queries SHOW ORGANIZATION ACCOUNTS to retrieve edition data (Standard, Enterprise, Business Critical)
  • Reports user count from SNOWFLAKE.ACCOUNT_USAGE.USERS as consumed seats
  • Gracefully skips license sync when ORGADMIN role is unavailable (logs warning, returns empty)
  • License resource type uses OptInRequired annotation so it must be explicitly enabled
  • Follows the same pattern used by baton-github's license implementation

Fixes: CXH-1935

Test plan

  • Verify the connector builds successfully (go build ./...)
  • Verify existing tests pass (go test ./...)
  • Test with a Snowflake account that has ORGADMIN role — license resources should appear with edition name and user count
  • Test with a Snowflake account without ORGADMIN — license sync should be skipped gracefully with a warning log
  • Verify the license resource has correct TRAIT_LICENSE_PROFILE annotations

Automated PR Notice

This PR was automatically created by c1-dev-bot as a potential implementation.

This code requires:

  • Human review of the implementation approach
  • Manual testing to verify correctness
  • Approval from the appropriate team before merging

Query Snowflake organization accounts via SHOW ORGANIZATION ACCOUNTS
to expose edition information (Standard, Enterprise, Business Critical)
as a license resource with TRAIT_LICENSE_PROFILE. User count from
ACCOUNT_USAGE.USERS is reported as consumed seats. Gracefully skips
license sync when ORGADMIN role is unavailable.
@c1-dev-bot c1-dev-bot Bot requested a review from a team June 29, 2026 16:41
@linear-code

linear-code Bot commented Jun 29, 2026

Copy link
Copy Markdown

CXH-1935

func (c *Client) ListOrganizationAccounts(ctx context.Context) ([]OrganizationAccount, int, error) {
queries := []string{"SHOW ORGANIZATION ACCOUNTS;"}

req, err := c.PostStatementRequest(ctx, queries)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Suggestion: SHOW ORGANIZATION ACCOUNTS requires the ORGADMIN role, but PostStatementRequest does not set the X-Snowflake-Role header, so the query runs with the JWT's default role. Unless ORGADMIN is the operator's default role, this will return 422 and the license sync will be silently skipped even for accounts that have ORGADMIN. Consider setting uhttp.WithHeader(RoleHeaderKey, "ORGADMIN") here (mirroring how user_rest.go sets RoleHeaderKey for USERADMIN). The same applies to CountUsers, which queries ACCOUNT_USAGE. (medium confidence)

Comment thread pkg/connector/license.go
rs.WithLicenseName(licenseName),
}
if userCount > 0 {
traitOpts = append(traitOpts, rs.WithLicenseSeats(0, userCount))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Suggestion: CountUsers queries SNOWFLAKE.ACCOUNT_USAGE.USERS, which is scoped to the single account the connector authenticates to, but this loop applies that same userCount as consumed seats to every organization account. In a multi-account org, non-current accounts will report an incorrect seat count. Consider only attaching seat data to the current account's license, or sourcing a per-account count. (medium confidence)

@github-actions

Copy link
Copy Markdown
Contributor

Connector PR Review: feat: add license resource type for Snowflake edition data

Blocking Issues: 0 | Suggestions: 3 | Threads Resolved: 0
Criteria: Criteria status: loaded .claude/skills/ci-review.md from trusted base 67dc4998a386.
Review mode: full
View review run

Review Summary

Scanned the full PR diff for security and correctness. The change adds a new license resource builder (TRAIT_LICENSE_PROFILE, opt-in via OptInRequired) plus two new client methods that query SHOW ORGANIZATION ACCOUNTS and a user count. The builder is registered unconditionally (correct — not gated on an API probe), HTTP body close / nil handling follows the existing pattern, and graceful 422 skip is in place. No security or blocking-correctness issues found; three non-blocking suggestions below.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

  • pkg/snowflake/organization.go:54 & :95 — Statement requests don't set X-Snowflake-Role, so ORGADMIN/ACCOUNT_USAGE queries run with the default role and license sync may be silently skipped even for capable accounts.
  • pkg/connector/license.go:61 — Single-account userCount is applied as consumed seats to every org account; incorrect for multi-account orgs.
  • New parsing paths (GetOrganizationAccounts, CountUsers) have no unit tests covering row parsing / count edge cases.
Prompt for AI agents
Verify each finding against the current code and only fix it if needed.

## Suggestions

In `pkg/snowflake/organization.go`:
- Around lines 51-55 and 92-95: The queries `SHOW ORGANIZATION ACCOUNTS` and the
  `SNOWFLAKE.ACCOUNT_USAGE.USERS` count require elevated roles (ORGADMIN / a role
  with ACCOUNT_USAGE access), but `PostStatementRequest` does not set the
  `X-Snowflake-Role` header, so the statement runs with the JWT's default role.
  Add a role header (e.g. pass an option that applies
  uhttp.WithHeader(RoleHeaderKey, "ORGADMIN")) the way user_rest.go sets
  RoleHeaderKey to UserAdminRole, so the sync works without requiring the
  operator's default role to already be ORGADMIN.

In `pkg/connector/license.go`:
- Around lines 41-61: CountUsers returns a count scoped to the single account the
  connector authenticates against, but the loop applies that same userCount as
  consumed seats to every account returned by ListOrganizationAccounts. For
  multi-account organizations this reports the wrong seat count on non-current
  accounts. Either attach seat data only to the current account's license, or
  obtain a per-account user count.

In `pkg/snowflake/organization.go`:
- Add table-driven unit tests for GetOrganizationAccounts (row parsing, missing
  edition) and CountUsers (empty data, non-numeric value) to cover the new
  parsing paths.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No blocking issues found.

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.

0 participants