nebari-dev / nebari-dev/llm-serving-pack

Consolidate *bool access helpers into a single IsPublic method

Open
#48 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

area: developer experience 👩🏻‍💻 good first issue needs: follow-up 📥 priority: low 🌱 type: maintenance 🛠
Dominant language
Go
Stars
3
Forks
3
Avg merge
16h 31m
Merged PRs (30d)
11

Description

Follow-up from #47.

Right now three spots compute "is this model public" differently:

  • operator/internal/controller/reconcilers/auth.go - isPublic(model) local helper.
  • operator/internal/controller/reconcilers/routing.go:17 - boolOrDefault(*bool, bool) bool, used for Endpoints.External.Enabled etc. but not Access.Public.
  • operator/internal/webhook/v1alpha1/llmmodel_webhook.go:130 - inline Public != nil && *Public inside validateAccess.

Each is correct in isolation; together they are three ways to express the same invariant on the same *bool field. Adding a fourth call site later is a coin flip for which form the author will pick.

Suggested shape

Add a method on the CRD type so both the operator and the webhook import a single source of truth:

// in api/v1alpha1/llmmodel_types.go
func (a AccessSpec) IsPublic() bool {
    return a.Public != nil && *a.Public
}

Then replace the three call sites with model.Spec.Access.IsPublic() and delete the local helper in auth.go.

Placing it on the type also makes it discoverable from the CRD doc and keeps the rule in the package that owns the field.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with api/v1alpha1/llmmodel_types.go, then inspect the helper and call sites in operator/internal/controller/reconcilers/auth.go, operator/internal/controller/reconcilers/routing.go, and operator/internal/webhook/v1alpha1/llmmodel_webhook.go. Confirm how each currently handles Access.Public and replace the three forms with the shared method. Done means the local helper and inline logic are removed and all three paths use the CRD method.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
api, backend
Issue type
Refactor
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.