Skip to content

Commit 428c50c

Browse files
author
baton-admin[bot]
committed
chore: add CHANGE_TYPES.md via baton-admin
1 parent 87d27aa commit 428c50c

File tree

1 file changed

+392
-0
lines changed

1 file changed

+392
-0
lines changed

CHANGE_TYPES.md

Lines changed: 392 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,392 @@
1+
# Change Type Detection and Guidance
2+
3+
When working on a connector, first identify what type of change you're making. Each type has specific patterns and pitfalls.
4+
5+
## Quick Reference
6+
7+
| Change Type | Detection Signal | Severity | Key Risk |
8+
|-------------|-----------------|----------|----------|
9+
| SDK Upgrade | `go.mod` baton-sdk version | Medium | Breaking changes |
10+
| Pagination Fix | `List()`, page tokens | **High** | Data loss or infinite loop |
11+
| Panic Fix | nil checks, type assertions | **High** | Production crashes |
12+
| Provisioning Fix | `Grant()`, `Revoke()` | **High** | Wrong entity source |
13+
| New Resource Type | new `*_builder.go` file | Medium | Breaking existing sync |
14+
| New Provisioning | adding Grant/Revoke methods | Medium | Missing ExternalId |
15+
| Linter Fix | style, formatting, lint errors | Low | None |
16+
| CI/Release Fix | goreleaser, workflows | Low | None |
17+
| Config Change | `config.go`, flags | Medium | Breaking existing deployments |
18+
19+
---
20+
21+
## SDK Upgrade
22+
23+
**Detection**: Changes to `go.mod` with `baton-sdk` version bump.
24+
25+
**Checklist**:
26+
- [ ] Read SDK changelog for breaking changes
27+
- [ ] Run full test suite (if exists)
28+
- [ ] Verify pagination still terminates
29+
- [ ] Check for deprecated function warnings
30+
- [ ] Test sync output matches previous version
31+
32+
**Common Issues**:
33+
- New required fields in ResourceBuilder
34+
- Changed pagination token handling
35+
- Renamed or moved interfaces
36+
37+
**Example**: Upgrading from v0.2.x to v0.4.x often requires:
38+
```go
39+
// Old: implicit resource type registration
40+
// New: explicit registration in ResourceTypes()
41+
func (c *Connector) ResourceTypes(ctx context.Context) ([]*v2.ResourceType, error) {
42+
return []*v2.ResourceType{userResourceType, groupResourceType}, nil
43+
}
44+
```
45+
46+
---
47+
48+
## Pagination Fix
49+
50+
**Detection**: Changes to `List()` methods, page token handling, or `pagination.Bag`.
51+
52+
**This is HIGH SEVERITY** - pagination bugs cause data loss (missing resources) or infinite loops (never-ending sync).
53+
54+
**Two Failure Modes**:
55+
56+
1. **Early Termination** - Sync stops too soon, missing pages:
57+
```go
58+
// WRONG - always returns empty token
59+
return resources, "", nil, nil
60+
```
61+
62+
2. **Infinite Loop** - Sync never stops:
63+
```go
64+
// WRONG - hardcoded token
65+
return resources, "next", nil, nil
66+
67+
// WRONG - nextPage never becomes empty
68+
return resources, resp.NextPage, nil, nil // if API returns cursor even on last page
69+
```
70+
71+
**Correct Pattern**:
72+
```go
73+
func (b *userBuilder) List(ctx context.Context, parentResourceID *v2.ResourceId, pt *pagination.Token) (
74+
[]*v2.Resource, string, annotations.Annotations, error,
75+
) {
76+
bag, page, err := parsePageToken(pt.Token, &v2.ResourceId{})
77+
if err != nil {
78+
return nil, "", nil, err
79+
}
80+
81+
// CRITICAL: Initialize bag on first call
82+
if bag.Current() == nil {
83+
bag.Push(pagination.PageState{ResourceTypeID: resourceTypeUser.Id})
84+
}
85+
86+
resp, err := c.client.ListUsers(ctx, page)
87+
if err != nil {
88+
return nil, "", nil, fmt.Errorf("baton-service: listing users: %w", err)
89+
}
90+
91+
// ... build resources ...
92+
93+
// CRITICAL: Only pass through API's token, detect termination
94+
if resp.NextPage == "" {
95+
return resources, "", nil, nil // Done
96+
}
97+
return resources, resp.NextPage, nil, nil // More pages
98+
}
99+
```
100+
101+
**Verification**:
102+
- [ ] Token comes from API response, not hardcoded
103+
- [ ] There exists a path where token becomes "" (termination)
104+
- [ ] Bag is initialized on first call (`if bag.Current() == nil`)
105+
- [ ] API's "no more pages" signal is correctly detected
106+
107+
---
108+
109+
## Panic Fix
110+
111+
**Detection**: Changes adding nil checks, fixing type assertions, or addressing runtime panics.
112+
113+
**This is HIGH SEVERITY** - panics crash production syncs.
114+
115+
**Common Panic Sources**:
116+
117+
1. **HTTP Response nil on error**:
118+
```go
119+
// WRONG - resp may be nil when err != nil
120+
resp, err := client.Do(req)
121+
if err != nil {
122+
log.Printf("status: %d", resp.StatusCode) // PANIC
123+
}
124+
125+
// CORRECT
126+
resp, err := client.Do(req)
127+
if err != nil {
128+
if resp != nil {
129+
defer resp.Body.Close()
130+
}
131+
return fmt.Errorf("baton-service: request failed: %w", err)
132+
}
133+
defer resp.Body.Close()
134+
```
135+
136+
2. **Type assertion without ok check**:
137+
```go
138+
// WRONG - panics if wrong type
139+
value := data["key"].(string)
140+
141+
// CORRECT
142+
value, ok := data["key"].(string)
143+
if !ok {
144+
return fmt.Errorf("baton-service: expected string for key")
145+
}
146+
```
147+
148+
3. **Pagination bag not initialized**:
149+
```go
150+
// WRONG - panics on first call
151+
token := bag.Current().Token
152+
153+
// CORRECT
154+
if bag.Current() == nil {
155+
bag.Push(pagination.PageState{ResourceTypeID: resourceType.Id})
156+
}
157+
```
158+
159+
4. **Nil ParentResourceId**:
160+
```go
161+
// WRONG - may panic
162+
orgID := resource.ParentResourceId.Resource
163+
164+
// CORRECT
165+
if resource.ParentResourceId == nil {
166+
return fmt.Errorf("baton-service: resource has no parent")
167+
}
168+
orgID := resource.ParentResourceId.Resource
169+
```
170+
171+
---
172+
173+
## Provisioning Fix
174+
175+
**Detection**: Changes to `Grant()`, `Revoke()`, or entity source logic.
176+
177+
**This is HIGH SEVERITY** - wrong provisioning can grant/revoke wrong access.
178+
179+
**The Entity Source Rule**:
180+
- **WHO** (principal): The user/service being granted access
181+
- **WHAT** (entitlement): The permission being granted
182+
- **WHERE** (context): The org/workspace scope - **ALWAYS from principal**
183+
184+
```go
185+
func (g *groupBuilder) Grant(ctx context.Context, principal *v2.Resource,
186+
entitlement *v2.Entitlement) ([]*v2.Grant, annotations.Annotations, error) {
187+
188+
// WHO - get native ID from ExternalId
189+
externalId := principal.GetExternalId()
190+
if externalId == nil {
191+
return nil, nil, fmt.Errorf("baton-service: principal missing external ID")
192+
}
193+
nativeUserID := externalId.Id
194+
195+
// WHAT - from entitlement
196+
groupID := entitlement.Resource.Id.Resource
197+
198+
// WHERE - CRITICAL: from principal, NOT entitlement
199+
workspaceID := principal.ParentResourceId.Resource // CORRECT
200+
// workspaceID := entitlement.Resource.ParentResourceId.Resource // WRONG
201+
202+
// Make API call
203+
err := c.client.AddMember(ctx, workspaceID, groupID, nativeUserID)
204+
// ...
205+
}
206+
```
207+
208+
**Idempotency**:
209+
```go
210+
// "Already exists" is SUCCESS, not error
211+
if isAlreadyExistsError(err) {
212+
return nil, annotations.New(&v2.GrantAlreadyExists{}), nil
213+
}
214+
215+
// "Not found" on revoke is SUCCESS
216+
if isNotFoundError(err) {
217+
return annotations.New(&v2.GrantAlreadyRevoked{}), nil
218+
}
219+
```
220+
221+
**Checklist**:
222+
- [ ] Context (workspace/org) comes from principal, not entitlement
223+
- [ ] ExternalId is used for API calls, not ResourceId
224+
- [ ] Already-exists handled as success
225+
- [ ] Not-found on revoke handled as success
226+
- [ ] Error messages include connector prefix
227+
228+
---
229+
230+
## New Resource Type
231+
232+
**Detection**: New `*_builder.go` file or new entry in `ResourceTypes()`.
233+
234+
**Breaking Change Risk**: Adding resource types is safe. Removing or renaming is NEVER safe.
235+
236+
**Checklist**:
237+
- [ ] Resource ID is stable (uses API ID, not mutable fields like email)
238+
- [ ] ResourceType is registered in `ResourceTypes()`
239+
- [ ] If no entitlements/grants, add `SkipEntitlementsAndGrants` annotation
240+
- [ ] ExternalId set if resource will be used in provisioning
241+
242+
**Template**:
243+
```go
244+
var resourceTypeWidget = &v2.ResourceType{
245+
Id: "widget",
246+
DisplayName: "Widget",
247+
Traits: []v2.ResourceType_Trait{v2.ResourceType_TRAIT_GROUP},
248+
Annotations: annotationsForResourceType("widget"),
249+
}
250+
251+
type widgetBuilder struct {
252+
client *Client
253+
}
254+
255+
func (b *widgetBuilder) ResourceType(ctx context.Context) *v2.ResourceType {
256+
return resourceTypeWidget
257+
}
258+
259+
func (b *widgetBuilder) List(ctx context.Context, parentResourceID *v2.ResourceId,
260+
pt *pagination.Token) ([]*v2.Resource, string, annotations.Annotations, error) {
261+
// Implementation
262+
}
263+
264+
func (b *widgetBuilder) Entitlements(ctx context.Context, resource *v2.Resource,
265+
pt *pagination.Token) ([]*v2.Entitlement, string, annotations.Annotations, error) {
266+
// Implementation or return nil, "", nil, nil if skipped
267+
}
268+
269+
func (b *widgetBuilder) Grants(ctx context.Context, resource *v2.Resource,
270+
pt *pagination.Token) ([]*v2.Grant, string, annotations.Annotations, error) {
271+
// Implementation or return nil, "", nil, nil if skipped
272+
}
273+
```
274+
275+
---
276+
277+
## New Provisioning
278+
279+
**Detection**: Adding `Grant()` or `Revoke()` methods to existing builder.
280+
281+
**Checklist**:
282+
- [ ] ExternalId is set during sync (required for provisioning to work)
283+
- [ ] Context comes from principal
284+
- [ ] Idempotency handled (already-exists, already-revoked)
285+
- [ ] capabilities.json updated if needed
286+
287+
**Critical**: If `WithExternalID()` wasn't set during sync, provisioning will fail:
288+
```go
289+
// During sync - REQUIRED for provisioning
290+
resource, err := rs.NewUserResource(
291+
user.Name,
292+
resourceTypeUser,
293+
user.ID,
294+
[]rs.UserTraitOption{rs.WithEmail(user.Email, true)},
295+
rs.WithExternalID(&v2.ExternalId{Id: user.NativeID}), // CRITICAL
296+
)
297+
```
298+
299+
---
300+
301+
## Linter Fix
302+
303+
**Detection**: Changes only to formatting, imports, or style.
304+
305+
**This is LOW SEVERITY** - but verify no logic changes snuck in.
306+
307+
**Common Linter Issues**:
308+
- Unused variables/imports
309+
- Error return not checked
310+
- Deprecated functions
311+
- Line length
312+
313+
**Checklist**:
314+
- [ ] Only formatting/style changes, no logic changes
315+
- [ ] Tests still pass (if any)
316+
317+
---
318+
319+
## CI/Release Fix
320+
321+
**Detection**: Changes to `.goreleaser.yml`, `.github/workflows/`, or build configuration.
322+
323+
**This is LOW SEVERITY** for connector logic, but can break releases.
324+
325+
**Common Issues**:
326+
- Go version mismatch between workflow and goreleaser
327+
- Missing environment variables
328+
- Changed artifact paths
329+
330+
**Checklist**:
331+
- [ ] Go version consistent across all configs
332+
- [ ] Release workflow triggers on correct events
333+
- [ ] Artifact names haven't changed (breaks downstream)
334+
335+
---
336+
337+
## Config Change
338+
339+
**Detection**: Changes to `config.go`, command-line flags, or environment variables.
340+
341+
**Breaking Change Risk**: Removing or renaming config fields breaks existing deployments.
342+
343+
**Checklist**:
344+
- [ ] New fields have sensible defaults (don't break existing configs)
345+
- [ ] Removed fields are deprecated first (warn, then remove)
346+
- [ ] Field names are consistent (`WithDisplayName` for all fields)
347+
- [ ] Validation added for required fields
348+
- [ ] Documentation updated
349+
350+
**Example - Adding a field safely**:
351+
```go
352+
type Config struct {
353+
Token string `mapstructure:"token"`
354+
BaseURL string `mapstructure:"base_url"`
355+
// New field with default - won't break existing configs
356+
PageSize int `mapstructure:"page_size"`
357+
}
358+
359+
func (c *Config) Validate() error {
360+
if c.Token == "" {
361+
return fmt.Errorf("token is required")
362+
}
363+
if c.PageSize == 0 {
364+
c.PageSize = 100 // Sensible default
365+
}
366+
return nil
367+
}
368+
```
369+
370+
---
371+
372+
## Detection Workflow
373+
374+
When starting work, identify the change type:
375+
376+
1. **Look at the files changed**:
377+
- `go.mod` -> SDK Upgrade
378+
- `*_builder.go` List methods -> Pagination
379+
- `*_builder.go` Grant/Revoke -> Provisioning
380+
- `config.go` -> Config Change
381+
- `.goreleaser.yml` -> CI/Release
382+
383+
2. **Look at the symptoms**:
384+
- "Sync hangs" -> Pagination (infinite loop)
385+
- "Missing resources" -> Pagination (early termination)
386+
- "Panic in production" -> Nil pointer or type assertion
387+
- "Wrong user got access" -> Entity source confusion
388+
- "Provisioning fails" -> Missing ExternalId
389+
390+
3. **Apply the relevant checklist** from this document.
391+
392+
4. **Test the specific risk** for that change type.

0 commit comments

Comments
 (0)