Fix Create methods: respect user-provided IDs and set timestamps

All service-layer Create methods (team, owner, target, issuer,
certificate) were unconditionally overwriting user-provided IDs with
auto-generated ones and leaving CreatedAt/UpdatedAt as zero values.

This caused three user-visible bugs:
- POST /api/v1/teams with {"id": "t-demo"} returned a generated ID
  like "team-1773601137949154216" instead of "t-demo"
- POST /api/v1/owners referencing the user-provided team_id failed
  with Internal Server Error (FK constraint on non-existent generated ID)
- created_at/updated_at came back as "0001-01-01T00:00:00Z"

Fix: all 9 affected Create methods (both context-aware and handler
interface variants) now check if ID is empty before generating, and
set timestamps to time.Now() if zero-valued. Follows the existing
correct pattern in policy.go CreateRule/CreatePolicy.

Also removes two stale temp files (audit.go.* and issuer.go.*) that
were accidentally committed to the repo.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Shankar
2026-03-15 14:52:48 -04:00
parent 18c4d36beb
commit 5463fbcc9f
7 changed files with 95 additions and 286 deletions
@@ -1,161 +0,0 @@
package service
import (
"context"
"encoding/json"
"fmt"
"time"
"github.com/shankar0123/certctl/internal/domain"
"github.com/shankar0123/certctl/internal/repository"
)
// AuditService provides business logic for recording and retrieving audit events.
type AuditService struct {
auditRepo repository.AuditRepository
}
// NewAuditService creates a new audit service.
func NewAuditService(auditRepo repository.AuditRepository) *AuditService {
return &AuditService{
auditRepo: auditRepo,
}
}
// RecordEvent records an audit event with actor, action, and resource information.
func (s *AuditService) RecordEvent(ctx context.Context, actor string, actorType domain.ActorType, action string, resourceType string, resourceID string, details map[string]interface{}) error {
detailsJSON, err := json.Marshal(details)
if err != nil {
detailsJSON = []byte("{}")
}
event := &domain.AuditEvent{
ID: generateID("audit"),
Timestamp: time.Now(),
Actor: actor,
ActorType: actorType,
Action: action,
ResourceType: resourceType,
ResourceID: resourceID,
Details: json.RawMessage(detailsJSON),
}
if err := s.auditRepo.Create(ctx, event); err != nil {
return fmt.Errorf("failed to record audit event: %w", err)
}
return nil
}
// List returns audit events matching filter criteria.
func (s *AuditService) List(ctx context.Context, filter *repository.AuditFilter) ([]*domain.AuditEvent, error) {
events, err := s.auditRepo.List(ctx, filter)
if err != nil {
return nil, fmt.Errorf("failed to list audit events: %w", err)
}
return events, nil
}
// ListByResource returns all audit events for a specific resource.
func (s *AuditService) ListByResource(ctx context.Context, resourceType string, resourceID string) ([]*domain.AuditEvent, error) {
filter := &repository.AuditFilter{
ResourceType: resourceType,
ResourceID: resourceID,
PerPage: 1000, // reasonable default for single resource
}
events, err := s.auditRepo.List(ctx, filter)
if err != nil {
return nil, fmt.Errorf("failed to list audit events: %w", err)
}
return events, nil
}
// ListByActor returns all audit events for a specific actor.
func (s *AuditService) ListByActor(ctx context.Context, actor string) ([]*domain.AuditEvent, error) {
filter := &repository.AuditFilter{
Actor: actor,
PerPage: 1000,
}
events, err := s.auditRepo.List(ctx, filter)
if err != nil {
return nil, fmt.Errorf("failed to list audit events: %w", err)
}
return events, nil
}
// ListByAction returns all audit events for a specific action type.
func (s *AuditService) ListByAction(ctx context.Context, action string, from, to time.Time) ([]*domain.AuditEvent, error) {
filter := &repository.AuditFilter{
From: from,
To: to,
PerPage: 1000,
}
events, err := s.auditRepo.List(ctx, filter)
if err != nil {
return nil, fmt.Errorf("failed to list audit events: %w", err)
}
// Filter by action on client side (repository may not filter by action directly)
var filtered []*domain.AuditEvent
for _, e := range events {
if e.Action == action {
filtered = append(filtered, e)
}
}
return filtered, nil
}
// ListAuditEvents returns paginated audit events (handler interface method).
func (s *AuditService) ListAuditEvents(page, perPage int) ([]domain.AuditEvent, int64, error) {
if page < 1 {
page = 1
}
if perPage < 1 {
perPage = 50
}
filter := &repository.AuditFilter{
Offset: int64((page - 1) * perPage),
PerPage: int64(perPage),
}
events, err := s.auditRepo.List(context.Background(), filter)
if err != nil {
return nil, 0, fmt.Errorf("failed to list audit events: %w", err)
}
// Convert pointers to values for the handler interface
var result []domain.AuditEvent
for _, e := range events {
if e != nil {
result = append(result, *e)
}
}
// TODO: Get total count from repository
total := int64(len(result))
return result, total, nil
}
// GetAuditEvent returns a single audit event (handler interface method).
func (s *AuditService) GetAuditEvent(id string) (*domain.AuditEvent, error) {
filter := &repository.AuditFilter{
ID: id,
}
events, err := s.auditRepo.List(context.Background(), filter)
if err != nil {
return nil, fmt.Errorf("failed to get audit event: %w", err)
}
if len(events) == 0 {
return nil, fmt.Errorf("audit event not found")
}
return events[0], nil
}
+11 -1
View File
@@ -3,6 +3,7 @@ package service
import (
"context"
"fmt"
"time"
"github.com/shankar0123/certctl/internal/domain"
"github.com/shankar0123/certctl/internal/repository"
@@ -254,7 +255,16 @@ func (s *CertificateService) GetCertificate(id string) (*domain.ManagedCertifica
// CreateCertificate creates a new certificate (handler interface method).
func (s *CertificateService) CreateCertificate(cert domain.ManagedCertificate) (*domain.ManagedCertificate, error) {
cert.ID = generateID("cert")
if cert.ID == "" {
cert.ID = generateID("cert")
}
now := time.Now()
if cert.CreatedAt.IsZero() {
cert.CreatedAt = now
}
if cert.UpdatedAt.IsZero() {
cert.UpdatedAt = now
}
if err := s.certRepo.Create(context.Background(), &cert); err != nil {
return nil, fmt.Errorf("failed to create certificate: %w", err)
}
+21 -2
View File
@@ -3,6 +3,7 @@ package service
import (
"context"
"fmt"
"time"
"github.com/shankar0123/certctl/internal/domain"
"github.com/shankar0123/certctl/internal/repository"
@@ -65,7 +66,16 @@ func (s *IssuerService) Create(ctx context.Context, issuer *domain.Issuer, actor
return fmt.Errorf("issuer name is required")
}
issuer.ID = generateID("issuer")
if issuer.ID == "" {
issuer.ID = generateID("issuer")
}
now := time.Now()
if issuer.CreatedAt.IsZero() {
issuer.CreatedAt = now
}
if issuer.UpdatedAt.IsZero() {
issuer.UpdatedAt = now
}
if err := s.issuerRepo.Create(ctx, issuer); err != nil {
return fmt.Errorf("failed to create issuer: %w", err)
}
@@ -160,7 +170,16 @@ func (s *IssuerService) GetIssuer(id string) (*domain.Issuer, error) {
// CreateIssuer creates a new issuer (handler interface method).
func (s *IssuerService) CreateIssuer(issuer domain.Issuer) (*domain.Issuer, error) {
issuer.ID = generateID("issuer")
if issuer.ID == "" {
issuer.ID = generateID("issuer")
}
now := time.Now()
if issuer.CreatedAt.IsZero() {
issuer.CreatedAt = now
}
if issuer.UpdatedAt.IsZero() {
issuer.UpdatedAt = now
}
if err := s.issuerRepo.Create(context.Background(), &issuer); err != nil {
return nil, fmt.Errorf("failed to create issuer: %w", err)
}
@@ -1,116 +0,0 @@
package service
import (
"context"
"fmt"
"github.com/shankar0123/certctl/internal/domain"
"github.com/shankar0123/certctl/internal/repository"
)
// IssuerService provides business logic for certificate issuer management.
type IssuerService struct {
issuerRepo repository.IssuerRepository
auditService *AuditService
}
// NewIssuerService creates a new issuer service.
func NewIssuerService(
issuerRepo repository.IssuerRepository,
auditService *AuditService,
) *IssuerService {
return &IssuerService{
issuerRepo: issuerRepo,
auditService: auditService,
}
}
// List returns a paginated list of issuers.
func (s *IssuerService) List(ctx context.Context, page, perPage int) ([]*domain.Issuer, int64, error) {
if page < 1 {
page = 1
}
if perPage < 1 {
perPage = 50
}
offset := int64((page - 1) * perPage)
issuers, total, err := s.issuerRepo.List(ctx, offset, int64(perPage))
if err != nil {
return nil, 0, fmt.Errorf("failed to list issuers: %w", err)
}
return issuers, total, nil
}
// Get retrieves an issuer by ID.
func (s *IssuerService) Get(ctx context.Context, id string) (*domain.Issuer, error) {
issuer, err := s.issuerRepo.Get(ctx, id)
if err != nil {
return nil, fmt.Errorf("failed to get issuer %s: %w", id, err)
}
return issuer, nil
}
// Create validates and stores a new issuer.
func (s *IssuerService) Create(ctx context.Context, issuer *domain.Issuer, actor string) error {
if issuer.Name == "" {
return fmt.Errorf("issuer name is required")
}
issuer.ID = generateID("issuer")
if err := s.issuerRepo.Create(ctx, issuer); err != nil {
return fmt.Errorf("failed to create issuer: %w", err)
}
if s.auditService != nil {
_ = s.auditService.RecordEvent(ctx, actor, domain.ActorTypeUser, "create_issuer", "issuer", issuer.ID, nil)
}
return nil
}
// Update modifies an existing issuer.
func (s *IssuerService) Update(ctx context.Context, id string, issuer *domain.Issuer, actor string) error {
if issuer.Name == "" {
return fmt.Errorf("issuer name is required")
}
issuer.ID = id
if err := s.issuerRepo.Update(ctx, issuer); err != nil {
return fmt.Errorf("failed to update issuer %s: %w", id, err)
}
if s.auditService != nil {
_ = s.auditService.RecordEvent(ctx, actor, domain.ActorTypeUser, "update_issuer", "issuer", id, nil)
}
return nil
}
// Delete removes an issuer.
func (s *IssuerService) Delete(ctx context.Context, id string, actor string) error {
if err := s.issuerRepo.Delete(ctx, id); err != nil {
return fmt.Errorf("failed to delete issuer %s: %w", id, err)
}
if s.auditService != nil {
_ = s.auditService.RecordEvent(ctx, actor, domain.ActorTypeUser, "delete_issuer", "issuer", id, nil)
}
return nil
}
// TestConnection verifies the issuer connection.
func (s *IssuerService) TestConnection(ctx context.Context, id string) error {
issuer, err := s.issuerRepo.Get(ctx, id)
if err != nil {
return fmt.Errorf("issuer not found: %w", err)
}
// TODO: Implement actual connection test based on issuer type
if issuer == nil {
return fmt.Errorf("issuer not found")
}
return nil
}
+21 -2
View File
@@ -3,6 +3,7 @@ package service
import (
"context"
"fmt"
"time"
"github.com/shankar0123/certctl/internal/domain"
"github.com/shankar0123/certctl/internal/repository"
@@ -65,7 +66,16 @@ func (s *OwnerService) Create(ctx context.Context, owner *domain.Owner, actor st
return fmt.Errorf("owner name is required")
}
owner.ID = generateID("owner")
if owner.ID == "" {
owner.ID = generateID("owner")
}
now := time.Now()
if owner.CreatedAt.IsZero() {
owner.CreatedAt = now
}
if owner.UpdatedAt.IsZero() {
owner.UpdatedAt = now
}
if err := s.ownerRepo.Create(ctx, owner); err != nil {
return fmt.Errorf("failed to create owner: %w", err)
}
@@ -140,7 +150,16 @@ func (s *OwnerService) GetOwner(id string) (*domain.Owner, error) {
// CreateOwner creates a new owner (handler interface method).
func (s *OwnerService) CreateOwner(owner domain.Owner) (*domain.Owner, error) {
owner.ID = generateID("owner")
if owner.ID == "" {
owner.ID = generateID("owner")
}
now := time.Now()
if owner.CreatedAt.IsZero() {
owner.CreatedAt = now
}
if owner.UpdatedAt.IsZero() {
owner.UpdatedAt = now
}
if err := s.ownerRepo.Create(context.Background(), &owner); err != nil {
return nil, fmt.Errorf("failed to create owner: %w", err)
}
+21 -2
View File
@@ -3,6 +3,7 @@ package service
import (
"context"
"fmt"
"time"
"github.com/shankar0123/certctl/internal/domain"
"github.com/shankar0123/certctl/internal/repository"
@@ -65,7 +66,16 @@ func (s *TargetService) Create(ctx context.Context, target *domain.DeploymentTar
return fmt.Errorf("target name is required")
}
target.ID = generateID("target")
if target.ID == "" {
target.ID = generateID("target")
}
now := time.Now()
if target.CreatedAt.IsZero() {
target.CreatedAt = now
}
if target.UpdatedAt.IsZero() {
target.UpdatedAt = now
}
if err := s.targetRepo.Create(ctx, target); err != nil {
return fmt.Errorf("failed to create target: %w", err)
}
@@ -140,7 +150,16 @@ func (s *TargetService) GetTarget(id string) (*domain.DeploymentTarget, error) {
// CreateTarget creates a new target (handler interface method).
func (s *TargetService) CreateTarget(target domain.DeploymentTarget) (*domain.DeploymentTarget, error) {
target.ID = generateID("target")
if target.ID == "" {
target.ID = generateID("target")
}
now := time.Now()
if target.CreatedAt.IsZero() {
target.CreatedAt = now
}
if target.UpdatedAt.IsZero() {
target.UpdatedAt = now
}
if err := s.targetRepo.Create(context.Background(), &target); err != nil {
return nil, fmt.Errorf("failed to create target: %w", err)
}
+21 -2
View File
@@ -3,6 +3,7 @@ package service
import (
"context"
"fmt"
"time"
"github.com/shankar0123/certctl/internal/domain"
"github.com/shankar0123/certctl/internal/repository"
@@ -65,7 +66,16 @@ func (s *TeamService) Create(ctx context.Context, team *domain.Team, actor strin
return fmt.Errorf("team name is required")
}
team.ID = generateID("team")
if team.ID == "" {
team.ID = generateID("team")
}
now := time.Now()
if team.CreatedAt.IsZero() {
team.CreatedAt = now
}
if team.UpdatedAt.IsZero() {
team.UpdatedAt = now
}
if err := s.teamRepo.Create(ctx, team); err != nil {
return fmt.Errorf("failed to create team: %w", err)
}
@@ -140,7 +150,16 @@ func (s *TeamService) GetTeam(id string) (*domain.Team, error) {
// CreateTeam creates a new team (handler interface method).
func (s *TeamService) CreateTeam(team domain.Team) (*domain.Team, error) {
team.ID = generateID("team")
if team.ID == "" {
team.ID = generateID("team")
}
now := time.Now()
if team.CreatedAt.IsZero() {
team.CreatedAt = now
}
if team.UpdatedAt.IsZero() {
team.UpdatedAt = now
}
if err := s.teamRepo.Create(context.Background(), &team); err != nil {
return nil, fmt.Errorf("failed to create team: %w", err)
}