Skip to content

chore(rest-api): Migrate Tenant and Fabric DAOs to use param structs#2308

Merged
nvlitagaki merged 8 commits into
NVIDIA:mainfrom
nvlitagaki:chore/dao-refactor
Jun 9, 2026
Merged

chore(rest-api): Migrate Tenant and Fabric DAOs to use param structs#2308
nvlitagaki merged 8 commits into
NVIDIA:mainfrom
nvlitagaki:chore/dao-refactor

Conversation

@nvlitagaki

@nvlitagaki nvlitagaki commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Description

This refactor updates the tenant and fabric DAOs to use more convenient structs for parameters rather than accepting a ton of different params.

Type of Change

  • Add - New feature or capability
  • Change - Changes in existing functionality
  • Fix - Bug fixes
  • Remove - Removed features or deprecated functionality
  • Internal - Internal changes (refactoring, tests, docs, etc.)

Related Issues (Optional)

Breaking Changes

  • This PR contains breaking changes

Testing

  • Unit tests added/updated
  • Integration tests added/updated
  • Manual testing performed
  • No testing required (docs, internal refactor, etc.)

Additional Notes

@nvlitagaki nvlitagaki requested a review from a team as a code owner June 8, 2026 18:09
@copy-pr-bot

copy-pr-bot Bot commented Jun 8, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@nvlitagaki nvlitagaki force-pushed the chore/dao-refactor branch from 7946eae to 0447072 Compare June 8, 2026 18:14
@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown

🔐 TruffleHog Secret Scan

✅ No secrets or credentials found!

Your code has been scanned for 700+ types of secrets and credentials. All clear! 🎉

🔗 View scan details

🕐 Last updated: 2026-06-08 18:16:47 UTC | Commit: 0447072

@nvlitagaki nvlitagaki marked this pull request as draft June 8, 2026 18:21
@copy-pr-bot

copy-pr-bot Bot commented Jun 8, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@nvlitagaki nvlitagaki force-pushed the chore/dao-refactor branch from 0447072 to adc2ff0 Compare June 8, 2026 18:34
@nvlitagaki nvlitagaki marked this pull request as ready for review June 8, 2026 18:36
@nvlitagaki nvlitagaki requested a review from thossain-nv June 8, 2026 18:37
@github-actions

github-actions Bot commented Jun 8, 2026 •
edited
Loading

Copy link
Copy Markdown

🔍 Container Scan Summary

Service Total Critical High Medium Low Other
nico-flow 116 13 50 41 4 8
nico-nsm 133 11 45 66 11 0
nico-psm 118 13 52 41 4 8
nico-rest-api 182 16 84 67 7 8
nico-rest-cert-manager 95 5 47 32 3 8
nico-rest-db 116 13 50 41 4 8
nico-rest-site-agent 115 13 50 41 3 8
nico-rest-site-manager 102 6 48 37 3 8
nico-rest-workflow 118 13 52 41 4 8
TOTAL 1095 103 478 407 43 64

Per-CVE detail lives in the per-service grype-* artifacts (JSON + SARIF). Severity counts only — no CVE IDs published here.

@coderabbitai

coderabbitai Bot commented Jun 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 4d33c047-9796-4346-b3ec-7ba3819edefd

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown

🌿 Preview your docs: https://nvidia-preview-pull-request-2308.docs.buildwithfern.com/infra-controller

thossain-nv
thossain-nv approved these changes Jun 8, 2026
View reviewed changes

@thossain-nv thossain-nv 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.

Looks good @nvlitagaki Left 2 suggestions.

Comment thread rest-api/db/pkg/db/model/tenant.go
@@ -96,11 +115,11 @@ type TenantDAO interface {
//
GetAllByOrg(ctx context.Context, tx *db.Tx, org string, includeRelations []string) ([]Tenant, error)

@thossain-nv thossain-nv Jun 8, 2026

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.

Should we convert this to a regular GetAll

@nvlitagaki nvlitagaki Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think not, at least not unless we see the need to fetch all tenants across orgs. Elsewhere in our code, GetAll tends to refer to functions where parameters are optional, whereas functions that require a particular parameter tend to be GetAllByX -- see getAllByEntityID in statusdetail.go and getAllByOrg in infrastructureprovider.org

Comment thread rest-api/api/pkg/api/handler/tenant.go Outdated
@thossain-nv thossain-nv changed the title chore(rest-api): migrate tenant and fabric DAOs to param structs chore(rest-api): Migrate Tenant and Fabric DAOs to use param structs Jun 8, 2026
nvlitagaki and others added 2 commits June 9, 2026 11:17
@nvlitagaki
fix(rest-api): respond to code review
67bb332
@poroh @nvlitagaki
test(nvue-client): read REPO_ROOT at runtime for config tests (NVIDIA…
c7c2e1c 
…#2309)

## Description

Avoid enforcing REPO_ROOT at compile time so checks like clippy can run
without requiring the environment variable during compilation.

## Type of Change
- [ ] **Add** - New feature or capability
- [ ] **Change** - Changes in existing functionality  
- [ ] **Fix** - Bug fixes
- [ ] **Remove** - Removed features or deprecated functionality
- [ ] **Internal** - Internal changes (refactoring, tests, docs, etc.)

## Related Issues (Optional)

NVIDIA#2257 

## Breaking Changes
- [ ] This PR contains breaking changes

## Testing
- [ ] Unit tests added/updated
- [ ] Integration tests added/updated  
- [ ] Manual testing performed
- [x] No testing required (docs, internal refactor, etc.)

## Additional Notes

Signed-off-by: Dmitry Porokh <dporokh@nvidia.com>
@nvlitagaki nvlitagaki force-pushed the chore/dao-refactor branch from 5166db8 to c7c2e1c Compare June 9, 2026 15:17
thossain-nv
thossain-nv reviewed Jun 9, 2026
View reviewed changes
Comment thread rest-api/api/pkg/api/handler/tenant.go
Name: userOrgDetails.Name,
DisplayName: &userOrgDetails.DisplayName,
Org: org,
OrgDisplayName: cutil.GetPtr(org),

@thossain-nv thossain-nv Jun 9, 2026

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.

This should be OrgDisplayName: &userOrgDetails.DisplayName,

hwadekar-nv
hwadekar-nv approved these changes Jun 9, 2026
View reviewed changes

@hwadekar-nv hwadekar-nv 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.

Looks good

@nvlitagaki nvlitagaki merged commit e867f98 into NVIDIA:main Jun 9, 2026
88 checks passed
@nvlitagaki nvlitagaki deleted the chore/dao-refactor branch June 9, 2026 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thossain-nv thossain-nv thossain-nv approved these changes

@hwadekar-nv hwadekar-nv hwadekar-nv approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@nvlitagaki @thossain-nv @hwadekar-nv @poroh