Skip to content

Commit 2387264

Browse files
committed
refactor: replace positional params with OrganizationUpdateOpts struct
Replace 8 positional parameters on OrganizationRepo.Update and OrganizationUseCase.Update with an OrganizationUpdateOpts struct for better readability and extensibility. Also regenerate CLI docs and add CLI documentation instruction to CLAUDE.md. Signed-off-by: Jose I. Paris <jiparis@chainloop.dev>
1 parent d20f8de commit 2387264

9 files changed

Lines changed: 80 additions & 73 deletions

File tree

CLAUDE.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,7 @@ Workflow Contracts define the structure and requirements for CI/CD attestations.
240240
- **Actions**: Implement business logic in `internal/action/`
241241
- **Policy Development**: Use `internal/policydevel/` for local policy testing
242242
- **Default Endpoints**: Override with `-ldflags` at build time using `defaultCASAPI` and `defaultCPAPI` variables
243+
- **Documentation**: After adding or modifying CLI commands, regenerate docs with `make generate` (runs `go generate ./...` which includes CLI docs)
243244

244245
### Artifact CAS Development
245246
- **Storage Backends**: Implement new backends in `pkg/blobmanager/` with Provider interface

app/cli/documentation/cli-reference.mdx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2824,6 +2824,7 @@ Options
28242824
```
28252825
--api-token-max-days-inactive string maximum days of inactivity before API tokens are auto-revoked (e.g. '90', '0' to disable)
28262826
--block set the default policy violation blocking strategy
2827+
--enable-ai-agent-collector enable automatic AI agent config collection during attestation init
28272828
-h, --help help for update
28282829
--name string organization name
28292830
--policies-allowed-hostnames strings set the allowed hostnames for the policy engine

app/controlplane/internal/service/organization.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,14 @@ func (s *OrganizationService) Update(ctx context.Context, req *pb.OrganizationSe
102102
apiTokenMaxDaysInactive = &days
103103
}
104104

105-
org, err := s.orgUC.Update(ctx, currentUser.ID, req.Name, req.BlockOnPolicyViolation, policiesAllowedHostnames, req.PreventImplicitWorkflowCreation, req.RestrictContractCreationToOrgAdmins, apiTokenMaxDaysInactive, req.EnableAiAgentCollector)
105+
org, err := s.orgUC.Update(ctx, currentUser.ID, req.Name, &biz.OrganizationUpdateOpts{
106+
BlockOnPolicyViolation: req.BlockOnPolicyViolation,
107+
PoliciesAllowedHostnames: policiesAllowedHostnames,
108+
PreventImplicitWorkflowCreation: req.PreventImplicitWorkflowCreation,
109+
RestrictContractCreationToOrgAdmins: req.RestrictContractCreationToOrgAdmins,
110+
APITokenInactivityThresholdDays: apiTokenMaxDaysInactive,
111+
EnableAIAgentCollector: req.EnableAiAgentCollector,
112+
})
106113
if err != nil {
107114
return nil, handleUseCaseErr(err, s.log)
108115
}

app/controlplane/pkg/biz/apitoken_stale_revoker_integration_test.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,9 @@ func (s *staleRevokerTestSuite) createOrgWithThreshold(ctx context.Context, days
199199
_, err = s.Membership.Create(ctx, org.ID, s.user.ID, biz.WithCurrentMembership())
200200
require.NoError(s.T(), err)
201201

202-
org, err = s.Organization.Update(ctx, s.user.ID, org.Name, nil, nil, nil, nil, &days, nil)
202+
org, err = s.Organization.Update(ctx, s.user.ID, org.Name, &biz.OrganizationUpdateOpts{
203+
APITokenInactivityThresholdDays: &days,
204+
})
203205
require.NoError(s.T(), err)
204206
require.NotNil(s.T(), org.APITokenInactivityThresholdDays)
205207
assert.Equal(s.T(), days, *org.APITokenInactivityThresholdDays)

app/controlplane/pkg/biz/mocks/OrganizationRepo.go

Lines changed: 15 additions & 45 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

app/controlplane/pkg/biz/organization.go

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,23 @@ type Organization struct {
5050
EnableAIAgentCollector bool
5151
}
5252

53+
// OrganizationUpdateOpts holds optional fields for updating an organization.
54+
// Pointer fields use nil to indicate "no change". For PoliciesAllowedHostnames,
55+
// nil means "no change" while an empty slice means "clear the list".
56+
type OrganizationUpdateOpts struct {
57+
BlockOnPolicyViolation *bool
58+
PoliciesAllowedHostnames []string
59+
PreventImplicitWorkflowCreation *bool
60+
RestrictContractCreationToOrgAdmins *bool
61+
APITokenInactivityThresholdDays *int
62+
EnableAIAgentCollector *bool
63+
}
64+
5365
type OrganizationRepo interface {
5466
FindByID(ctx context.Context, orgID uuid.UUID) (*Organization, error)
5567
FindByName(ctx context.Context, name string) (*Organization, error)
5668
Create(ctx context.Context, name string) (*Organization, error)
57-
Update(ctx context.Context, id uuid.UUID, blockOnPolicyViolation *bool, policiesAllowedHostnames []string, preventImplicitWorkflowCreation *bool, restrictContractCreationToOrgAdmins *bool, apiTokenInactivityThresholdDays *int, enableAIAgentCollector *bool) (*Organization, error)
69+
Update(ctx context.Context, id uuid.UUID, opts *OrganizationUpdateOpts) (*Organization, error)
5870
Delete(ctx context.Context, ID uuid.UUID) error
5971
// FindWithTokenInactivityThreshold returns orgs that have api_token_inactivity_threshold_days set (non-nil).
6072
FindWithTokenInactivityThreshold(ctx context.Context) ([]*Organization, error)
@@ -196,7 +208,7 @@ func (uc *OrganizationUseCase) doCreate(ctx context.Context, name string, opts .
196208
return org, nil
197209
}
198210

199-
func (uc *OrganizationUseCase) Update(ctx context.Context, userID, orgName string, blockOnPolicyViolation *bool, policiesAllowedHostnames []string, preventImplicitWorkflowCreation *bool, restrictContractCreationToOrgAdmins *bool, apiTokenInactivityThresholdDays *int, enableAIAgentCollector *bool) (*Organization, error) {
211+
func (uc *OrganizationUseCase) Update(ctx context.Context, userID, orgName string, opts *OrganizationUpdateOpts) (*Organization, error) {
200212
userUUID, err := uuid.Parse(userID)
201213
if err != nil {
202214
return nil, NewErrInvalidUUID(err)
@@ -216,7 +228,7 @@ func (uc *OrganizationUseCase) Update(ctx context.Context, userID, orgName strin
216228
}
217229

218230
// Perform the update
219-
org, err := uc.orgRepo.Update(ctx, orgUUID, blockOnPolicyViolation, policiesAllowedHostnames, preventImplicitWorkflowCreation, restrictContractCreationToOrgAdmins, apiTokenInactivityThresholdDays, enableAIAgentCollector)
231+
org, err := uc.orgRepo.Update(ctx, orgUUID, opts)
220232
if err != nil {
221233
return nil, fmt.Errorf("failed to update organization: %w", err)
222234
} else if org == nil {

app/controlplane/pkg/biz/organization_integration_test.go

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -118,53 +118,65 @@ func (s *OrgIntegrationTestSuite) TestUpdate() {
118118

119119
s.Run("org non existent", func() {
120120
// org not found
121-
_, err := s.Organization.Update(ctx, s.user.ID, uuid.NewString(), nil, nil, nil, nil, nil, nil)
121+
_, err := s.Organization.Update(ctx, s.user.ID, uuid.NewString(), &biz.OrganizationUpdateOpts{})
122122
s.Error(err)
123123
s.True(biz.IsNotFound(err))
124124
})
125125

126126
s.Run("org not accessible to user", func() {
127127
org2, err := s.Organization.CreateWithRandomName(ctx)
128128
require.NoError(s.T(), err)
129-
_, err = s.Organization.Update(ctx, s.user.ID, org2.Name, nil, nil, nil, nil, nil, nil)
129+
_, err = s.Organization.Update(ctx, s.user.ID, org2.Name, &biz.OrganizationUpdateOpts{})
130130
s.Error(err)
131131
s.True(biz.IsNotFound(err))
132132
})
133133

134134
s.Run("valid block on policy violation update", func() {
135-
got, err := s.Organization.Update(ctx, s.user.ID, s.org.Name, toPtrBool(true), nil, nil, nil, nil, nil)
135+
got, err := s.Organization.Update(ctx, s.user.ID, s.org.Name, &biz.OrganizationUpdateOpts{
136+
BlockOnPolicyViolation: toPtrBool(true),
137+
})
136138
s.NoError(err)
137139
s.True(got.BlockOnPolicyViolation)
138140
})
139141

140142
s.Run("valid policy allowed hostnames update", func() {
141-
got, err := s.Organization.Update(ctx, s.user.ID, s.org.Name, nil, []string{"foo.com", "bar.com"}, nil, nil, nil, nil)
143+
got, err := s.Organization.Update(ctx, s.user.ID, s.org.Name, &biz.OrganizationUpdateOpts{
144+
PoliciesAllowedHostnames: []string{"foo.com", "bar.com"},
145+
})
142146
s.NoError(err)
143147
s.Equal([]string{"foo.com", "bar.com"}, got.PoliciesAllowedHostnames)
144148
})
145149

146150
s.Run("clear policy allowed hostnames", func() {
147-
got, err := s.Organization.Update(ctx, s.user.ID, s.org.Name, nil, []string{}, nil, nil, nil, nil)
151+
got, err := s.Organization.Update(ctx, s.user.ID, s.org.Name, &biz.OrganizationUpdateOpts{
152+
PoliciesAllowedHostnames: []string{},
153+
})
148154
s.NoError(err)
149155
s.Equal([]string{}, got.PoliciesAllowedHostnames)
150156
})
151157

152158
s.Run("valid enable AI agent collector update", func() {
153-
got, err := s.Organization.Update(ctx, s.user.ID, s.org.Name, nil, nil, nil, nil, nil, toPtrBool(true))
159+
got, err := s.Organization.Update(ctx, s.user.ID, s.org.Name, &biz.OrganizationUpdateOpts{
160+
EnableAIAgentCollector: toPtrBool(true),
161+
})
154162
s.NoError(err)
155163
s.True(got.EnableAIAgentCollector)
156164

157-
got, err = s.Organization.Update(ctx, s.user.ID, s.org.Name, nil, nil, nil, nil, nil, toPtrBool(false))
165+
got, err = s.Organization.Update(ctx, s.user.ID, s.org.Name, &biz.OrganizationUpdateOpts{
166+
EnableAIAgentCollector: toPtrBool(false),
167+
})
158168
s.NoError(err)
159169
s.False(got.EnableAIAgentCollector)
160170
})
161171

162172
s.Run("but not passing a value doesn't clear the hostnames value", func() {
163-
got, err := s.Organization.Update(ctx, s.user.ID, s.org.Name, nil, []string{"foo.com", "bar.com"}, nil, nil, nil, nil)
173+
got, err := s.Organization.Update(ctx, s.user.ID, s.org.Name, &biz.OrganizationUpdateOpts{
174+
PoliciesAllowedHostnames: []string{"foo.com", "bar.com"},
175+
})
164176
s.NoError(err)
165177
s.Equal([]string{"foo.com", "bar.com"}, got.PoliciesAllowedHostnames)
166178

167-
got, err = s.Organization.Update(ctx, s.user.ID, s.org.Name, nil, nil, nil, nil, nil, nil)
179+
got, err = s.Organization.Update(ctx, s.user.ID, s.org.Name, &biz.OrganizationUpdateOpts{})
168180
s.NoError(err)
169181
s.Equal([]string{"foo.com", "bar.com"}, got.PoliciesAllowedHostnames)
170182
})

app/controlplane/pkg/biz/workflow_integration_test.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,9 @@ func (s *workflowIntegrationTestSuite) TestCreate() {
187187

188188
// Enable implicit workflow creation prevention for testing
189189
orgID, _ := uuid.Parse(s.org.ID)
190-
_, err = s.Repos.OrganizationRepo.Update(ctx, orgID, nil, nil, toPtrBool(true), nil, nil, nil)
190+
_, err = s.Repos.OrganizationRepo.Update(ctx, orgID, &biz.OrganizationUpdateOpts{
191+
PreventImplicitWorkflowCreation: toPtrBool(true),
192+
})
191193
s.Require().NoError(err)
192194

193195
for _, tc := range testCases {

app/controlplane/pkg/data/organization.go

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -77,28 +77,28 @@ func (r *OrganizationRepo) FindByName(ctx context.Context, name string) (*biz.Or
7777
return entOrgToBizOrg(org), nil
7878
}
7979

80-
func (r *OrganizationRepo) Update(ctx context.Context, id uuid.UUID, blockOnPolicyViolation *bool, policiesAllowedHostnames []string, preventImplicitWorkflowCreation *bool, restrictContractCreationToOrgAdmins *bool, apiTokenInactivityThresholdDays *int, enableAIAgentCollector *bool) (*biz.Organization, error) {
81-
opts := r.data.DB.Organization.UpdateOneID(id).
80+
func (r *OrganizationRepo) Update(ctx context.Context, id uuid.UUID, updateOpts *biz.OrganizationUpdateOpts) (*biz.Organization, error) {
81+
query := r.data.DB.Organization.UpdateOneID(id).
8282
Where(organization.DeletedAtIsNil()).
83-
SetNillableBlockOnPolicyViolation(blockOnPolicyViolation).
84-
SetNillablePreventImplicitWorkflowCreation(preventImplicitWorkflowCreation).
85-
SetNillableRestrictContractCreationToOrgAdmins(restrictContractCreationToOrgAdmins).
86-
SetNillableEnableAiAgentCollector(enableAIAgentCollector).
83+
SetNillableBlockOnPolicyViolation(updateOpts.BlockOnPolicyViolation).
84+
SetNillablePreventImplicitWorkflowCreation(updateOpts.PreventImplicitWorkflowCreation).
85+
SetNillableRestrictContractCreationToOrgAdmins(updateOpts.RestrictContractCreationToOrgAdmins).
86+
SetNillableEnableAiAgentCollector(updateOpts.EnableAIAgentCollector).
8787
SetUpdatedAt(time.Now())
8888

89-
if policiesAllowedHostnames != nil {
90-
opts.SetPoliciesAllowedHostnames(policiesAllowedHostnames)
89+
if updateOpts.PoliciesAllowedHostnames != nil {
90+
query.SetPoliciesAllowedHostnames(updateOpts.PoliciesAllowedHostnames)
9191
}
9292

93-
if apiTokenInactivityThresholdDays != nil {
94-
if *apiTokenInactivityThresholdDays == 0 {
95-
opts.ClearAPITokenInactivityThresholdDays()
93+
if updateOpts.APITokenInactivityThresholdDays != nil {
94+
if *updateOpts.APITokenInactivityThresholdDays == 0 {
95+
query.ClearAPITokenInactivityThresholdDays()
9696
} else {
97-
opts.SetAPITokenInactivityThresholdDays(*apiTokenInactivityThresholdDays)
97+
query.SetAPITokenInactivityThresholdDays(*updateOpts.APITokenInactivityThresholdDays)
9898
}
9999
}
100100

101-
org, err := opts.Save(ctx)
101+
org, err := query.Save(ctx)
102102
if err != nil {
103103
return nil, fmt.Errorf("failed to update organization: %w", err)
104104
}

0 commit comments

Comments
 (0)