Discharge7443/fleet
1
0
Fork 0
Code Issues Pull Requests Actions 5 Packages Projects Releases Wiki Activity

Fixed GitOps failing to delete a certificate authority (#41693)

Browse Source 
<!-- Add the related story/sub-task/bug number, like Resolves #123, or
remove if NA -->
**Related issue:** Resolves #38036

# Checklist for submitter

If some of the following don't apply, delete the relevant line.

- [x] Changes file added for user-visible changes in `changes/`,
`orbit/changes/` or `ee/fleetd-chrome/changes`.

## Testing

- [x] Added/updated automated tests
- [x] QA'd all new/changed functionality manually

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Bug Fixes**
* GitOps now correctly orders operations so certificate authorities can
be removed only after referencing certificate templates are handled,
preventing failed deletions during config updates.
* Improved user-facing error when a CA cannot be deleted because
certificate templates still reference it, with guidance to remove
templates first.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Victor Lyuboslavsky
2026-03-16 15:51:28 -05:00
committed by GitHub
parent b7b5d4190e
commit 902b4af289
14 changed files with 248 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/datastore/mysql/certificate_authorities.go
+8 -3
View File
@@ -350,6 +350,11 @@ func batchDeleteCertificateAuthorities(ctx context.Context, tx sqlx.ExtContext,
_, err := tx.ExecContext(ctx, stmt, args...)
if err != nil {
if isMySQLForeignKey(err) {
return &fleet.ConflictError{
Message: "Couldn't delete certificate authority. " + fleet.DeleteCAReferencedByTemplatesErrMsg + ". Please remove the certificate templates first.",
}
}
return ctxerr.Wrap(ctx, err, "deleting certificate authorities")
}
@@ -363,10 +368,10 @@ func (ds *Datastore) BatchApplyCertificateAuthorities(ctx context.Context, ops f
upserts = append(upserts, ops.Update...)
return ds.withRetryTxx(ctx, func(tx sqlx.ExtContext) error {
if err := batchDeleteCertificateAuthorities(ctx, tx, ops.Delete); err != nil {
if err := batchUpsertCertificateAuthorities(ctx, tx, ds.serverPrivateKey, upserts); err != nil {
return err
}
if err := batchUpsertCertificateAuthorities(ctx, tx, ds.serverPrivateKey, upserts); err != nil {
if err := batchDeleteCertificateAuthorities(ctx, tx, ops.Delete); err != nil {
return err
}
return nil
@@ -396,7 +401,7 @@ func (ds *Datastore) DeleteCertificateAuthority(ctx context.Context, certificate
if err != nil {
if isMySQLForeignKey(err) {
return nil, fleet.ConflictError{
Message: "Couldn't delete. This certificate authority is used in a certificate. Please remove the certificate first.",
Message: "Couldn't delete certificate authority. " + fleet.DeleteCAReferencedByTemplatesErrMsg + ". Please remove the certificate templates first.",
}
}
return nil, ctxerr.Wrap(ctx, err, fmt.Sprintf("deleting certificate authority with id %d", certificateAuthorityID))
server/datastore/mysql/certificate_authorities_test.go
+1 -1
View File
@@ -402,7 +402,7 @@ func testDeleteCertificateAuthority(t *testing.T, ds *Datastore) {
require.Error(t, err)
var conflictErr fleet.ConflictError
require.ErrorAs(t, err, &conflictErr)
require.Contains(t, conflictErr.Error(), "certificate authority is used in a certificate")
require.Contains(t, conflictErr.Error(), fleet.DeleteCAReferencedByTemplatesErrMsg)
}
func testUpdateCertificateAuthorityByID(t *testing.T, ds *Datastore) {
server/fleet/certificate_authorities.go
+7
View File
@@ -701,6 +701,13 @@ func ValidateCertificateAuthoritiesSpec(incoming interface{}) (*GroupedCertifica
return &groupedCAs, nil
}
// BatchApplyCertificateAuthoritiesOpts controls which operations the batch apply endpoint performs.
type BatchApplyCertificateAuthoritiesOpts struct {
DryRun bool
ViaGitOps bool
SkipDeletes bool // Process creates/updates only; skip deletions.
}
// CertificateAuthoritiesBatchOperations groups the operations for batch processing of certificate authorities.
type CertificateAuthoritiesBatchOperations struct {
Delete []*CertificateAuthority
server/fleet/errors.go
+5
View File
@@ -534,6 +534,11 @@ var (
SCEPRenewalIDWithoutURLChallengeErrMsg = "Variable \"$FLEET_VAR_" + string(FleetVarSCEPRenewalID) + "\" can't be used if variables for SCEP URL and Challenge are not specified."
)
const (
// DeleteCAReferencedByTemplatesErrMsg is the error substring used when a CA cannot be deleted because certificate templates still reference it.
DeleteCAReferencedByTemplatesErrMsg = "Certificate templates still reference it"
)
// ConflictError is used to indicate a conflict, such as a UUID conflict in the DB.
type ConflictError struct {
Message string
server/fleet/service.go
+1 -1
View File
@@ -1441,7 +1441,7 @@ type Service interface {
UpdateCertificateAuthority(ctx context.Context, id uint, p CertificateAuthorityUpdatePayload) error
RequestCertificate(ctx context.Context, p RequestCertificatePayload) (*string, error)
// BatchApplyCertificateAuthorities applies the given certificate authorities spec
BatchApplyCertificateAuthorities(ctx context.Context, groupedCAs GroupedCertificateAuthorities, dryRun bool, viaGitOps bool) error
BatchApplyCertificateAuthorities(ctx context.Context, groupedCAs GroupedCertificateAuthorities, opts BatchApplyCertificateAuthoritiesOpts) error
// GetGroupedCertificateAuthorities retrieves the grouped certificate authorities
GetGroupedCertificateAuthorities(ctx context.Context, includeSecrets bool) (*GroupedCertificateAuthorities, error)
server/mock/service/service_mock.go
+3 -3
View File
@@ -885,7 +885,7 @@ type UpdateCertificateAuthorityFunc func(ctx context.Context, id uint, p fleet.C
type RequestCertificateFunc func(ctx context.Context, p fleet.RequestCertificatePayload) (*string, error)
type BatchApplyCertificateAuthoritiesFunc func(ctx context.Context, groupedCAs fleet.GroupedCertificateAuthorities, dryRun bool, viaGitOps bool) error
type BatchApplyCertificateAuthoritiesFunc func(ctx context.Context, groupedCAs fleet.GroupedCertificateAuthorities, opts fleet.BatchApplyCertificateAuthoritiesOpts) error
type GetGroupedCertificateAuthoritiesFunc func(ctx context.Context, includeSecrets bool) (*fleet.GroupedCertificateAuthorities, error)
@@ -5234,11 +5234,11 @@ func (s *Service) RequestCertificate(ctx context.Context, p fleet.RequestCertifi
return s.RequestCertificateFunc(ctx, p)
}
func (s *Service) BatchApplyCertificateAuthorities(ctx context.Context, groupedCAs fleet.GroupedCertificateAuthorities, dryRun bool, viaGitOps bool) error {
func (s *Service) BatchApplyCertificateAuthorities(ctx context.Context, groupedCAs fleet.GroupedCertificateAuthorities, opts fleet.BatchApplyCertificateAuthoritiesOpts) error {
s.mu.Lock()
s.BatchApplyCertificateAuthoritiesFuncInvoked = true
s.mu.Unlock()
return s.BatchApplyCertificateAuthoritiesFunc(ctx, groupedCAs, dryRun, viaGitOps)
return s.BatchApplyCertificateAuthoritiesFunc(ctx, groupedCAs, opts)
}
func (s *Service) GetGroupedCertificateAuthorities(ctx context.Context, includeSecrets bool) (*fleet.GroupedCertificateAuthorities, error) {
server/service/certificate_authorities.go
+7 -3
View File
@@ -177,6 +177,7 @@ func (svc *Service) RequestCertificate(ctx context.Context, p fleet.RequestCerti
type batchApplyCertificateAuthoritiesRequest struct {
CertificateAuthorities fleet.GroupedCertificateAuthorities `json:"certificate_authorities"`
DryRun bool `json:"dry_run"`
SkipDeletes bool `json:"skip_deletes"`
}
// TODO(hca): do we need to return anything to facilitate logging by the gitops client?
@@ -189,8 +190,11 @@ func (r batchApplyCertificateAuthoritiesResponse) Error() error { return r.Err }
func batchApplyCertificateAuthoritiesEndpoint(ctx context.Context, request interface{}, svc fleet.Service) (fleet.Errorer, error) {
req := request.(*batchApplyCertificateAuthoritiesRequest)
// Call the service method to apply the certificate authorities spec
err := svc.BatchApplyCertificateAuthorities(ctx, req.CertificateAuthorities, req.DryRun, true)
err := svc.BatchApplyCertificateAuthorities(ctx, req.CertificateAuthorities, fleet.BatchApplyCertificateAuthoritiesOpts{
DryRun: req.DryRun,
ViaGitOps: true,
SkipDeletes: req.SkipDeletes,
})
if err != nil {
return &batchApplyCertificateAuthoritiesResponse{Err: err}, nil
}
@@ -198,7 +202,7 @@ func batchApplyCertificateAuthoritiesEndpoint(ctx context.Context, request inter
return &batchApplyCertificateAuthoritiesResponse{}, nil
}
func (svc *Service) BatchApplyCertificateAuthorities(ctx context.Context, incoming fleet.GroupedCertificateAuthorities, dryRun bool, viaGitOps bool) error {
func (svc *Service) BatchApplyCertificateAuthorities(ctx context.Context, incoming fleet.GroupedCertificateAuthorities, opts fleet.BatchApplyCertificateAuthoritiesOpts) error {
if err := svc.authz.Authorize(ctx, &fleet.CertificateAuthority{}, fleet.ActionWrite); err != nil {
return err
}
server/service/client.go
+3 -1
View File
@@ -578,7 +578,9 @@ func (c *Client) ApplyGroup(
}
if specs.CertificateAuthorities != nil {
if err := c.ApplyCertificateAuthoritiesSpec(*specs.CertificateAuthorities, opts.ApplySpecOptions); err != nil {
// In GitOps, skip deletes here. CA deletions are deferred to a post-op so that team configs
// can clean up certificate templates (which have FK references to CAs) first.
if err := c.ApplyCertificateAuthoritiesSpec(*specs.CertificateAuthorities, opts.ApplySpecOptions, fleet.BatchApplyCertificateAuthoritiesOpts{SkipDeletes: viaGitOps}); err != nil {
// only do this custom message for gitops as we reference the applying filename which only makes sense in gitops
if err.Error() == "missing or invalid license" && viaGitOps && filename != nil {
return nil, nil, nil, nil, fmt.Errorf("Couldn't edit \"%s\" at \"certificate_authorities\": Missing or invalid license. Certificate authorities are available in Fleet Premium only.", *filename)
server/service/client_certificate_authorities.go
+7 -3
View File
@@ -15,11 +15,15 @@ func (c *Client) GetCertificateAuthoritiesSpec(includeSecrets bool) (*fleet.Grou
}
// ApplyCertificateAuthoritiesSpec applies the certificate authorities.
func (c *Client) ApplyCertificateAuthoritiesSpec(groupedCAs fleet.GroupedCertificateAuthorities, opts fleet.ApplySpecOptions) error {
req := batchApplyCertificateAuthoritiesRequest{CertificateAuthorities: groupedCAs, DryRun: opts.DryRun}
func (c *Client) ApplyCertificateAuthoritiesSpec(groupedCAs fleet.GroupedCertificateAuthorities, specOpts fleet.ApplySpecOptions, opts fleet.BatchApplyCertificateAuthoritiesOpts) error {
req := batchApplyCertificateAuthoritiesRequest{
CertificateAuthorities: groupedCAs,
DryRun: specOpts.DryRun,
SkipDeletes: opts.SkipDeletes,
}
verb, path := "POST", "/api/latest/fleet/spec/certificate_authorities"
var responseBody batchApplyCertificateAuthoritiesResponse
return c.authenticatedRequestWithQuery(req, verb, path, &responseBody, opts.RawQuery())
return c.authenticatedRequestWithQuery(req, verb, path, &responseBody, specOpts.RawQuery())
}
// GetCertificateAuthorities fetches the list of certificate authorities