Skip to content

Commit 18d9497

Browse files
authored
fix: scorecard token-permissions and AI code quality findings (#289)
* fix: scorecard token-permissions and AI code quality findings - Move packages:write from top-level to job-level in mirror-e2e-images workflow (fixes Scorecard Token-Permissions score of 0) - Fix misleading error message: "mode is Canary" -> "updateStrategy.type is Canary" in webhook validation (matches actual field name) - Use apierrors.IsNotFound(err) instead of err != nil in integration test for policy deletion (prevents false positives on transient API errors) - Update test assertion and docs to match corrected error message Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca> * ci: exclude cert-manager.io from link checker (transient timeouts) Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca> --------- Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
1 parent 4857672 commit 18d9497

7 files changed

Lines changed: 12 additions & 8 deletions

File tree

.github/workflows/mirror-e2e-images.yaml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ on:
2020

2121
permissions:
2222
contents: read
23-
packages: write
2423

2524
concurrency:
2625
group: mirror-e2e-images
@@ -31,6 +30,9 @@ jobs:
3130
name: Mirror cert-manager to GHCR
3231
runs-on: ${{ vars.RUNNER || 'ubuntu-latest' }}
3332
timeout-minutes: 10
33+
permissions:
34+
contents: read
35+
packages: write
3436
steps:
3537
- uses: step-security/harden-runner@9af89fc71515a100421586dfdb3dc9c984fbf411 # v2.19.4
3638
with:

docs/SPEC.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -313,9 +313,9 @@ x-kubernetes-validations:
313313
- rule: "!has(self.minAllowed) || !has(self.maxAllowed) || self.minAllowed <= self.maxAllowed"
314314
message: "memory.minAllowed must be less than or equal to memory.maxAllowed"
315315

316-
# updateStrategy: canary config required when mode is Canary
316+
# updateStrategy: canary config required when type is Canary
317317
- rule: "self.updateStrategy.type != 'Canary' || has(self.updateStrategy.canary)"
318-
message: "canary configuration is required when mode is Canary"
318+
message: "canary configuration is required when updateStrategy.type is Canary"
319319

320320
# weight: immutable after creation (prevents runtime priority races)
321321
- rule: "self.weight == oldSelf.weight"

docs/reference/api.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ spec:
9999
updateStrategy:
100100
type: Recommend # Observe | Recommend | OneShot | Canary | Auto
101101
initialSizing: false # optional: set pod resources at creation time via webhook
102-
canary: # required when mode is Canary
102+
canary: # required when type is Canary
103103
percentage: 10 # % of pods to resize first
104104
observationPeriod: 30m # watch canary pods before proceeding (minimum: 1m)
105105
autoPromote: true # promote to full fleet after safe observation (default: false)

internal/webhook/validation.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,9 +97,9 @@ func (v *AttunePolicyValidator) validate(policy *attunev1alpha1.AttunePolicy) (a
9797
}
9898
}
9999

100-
// Canary config required when mode is Canary
100+
// Canary config required when type is Canary
101101
if us.Type == attunev1alpha1.UpdateTypeCanary && us.Canary == nil {
102-
return warnings, fmt.Errorf("updateStrategy.canary is required when mode is Canary")
102+
return warnings, fmt.Errorf("updateStrategy.canary is required when updateStrategy.type is Canary")
103103
}
104104

105105
// Validate canary observation period has a minimum floor.

internal/webhook/validation_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ func TestValidate_CanaryModeWithoutConfig(t *testing.T) {
222222
warnings, err := validator.ValidateCreate(context.Background(), policy)
223223

224224
assert.Error(t, err)
225-
assert.Contains(t, err.Error(), "updateStrategy.canary is required when mode is Canary")
225+
assert.Contains(t, err.Error(), "updateStrategy.canary is required when updateStrategy.type is Canary")
226226
assert.Empty(t, warnings)
227227
}
228228

lychee.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ exclude = [
1313
"instagram\\.com",
1414
"cast\\.ai",
1515
"helm\\.sh",
16+
"cert-manager\\.io",
1617
]
1718
exclude_loopback = true
1819
max_concurrency = 8

test/integration/controller_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import (
3232
"github.com/stretchr/testify/require"
3333
appsv1 "k8s.io/api/apps/v1"
3434
corev1 "k8s.io/api/core/v1"
35+
apierrors "k8s.io/apimachinery/pkg/api/errors"
3536
"k8s.io/apimachinery/pkg/api/resource"
3637
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3738
"k8s.io/apimachinery/pkg/types"
@@ -422,7 +423,7 @@ func TestReconcile_DeletedPolicy_NoError(t *testing.T) {
422423
Name: "policy-delete",
423424
Namespace: namespace,
424425
}, &fetched)
425-
return err != nil // should be NotFound
426+
return apierrors.IsNotFound(err)
426427
}, 30*time.Second, 500*time.Millisecond, "policy should be deleted")
427428
}
428429

0 commit comments

Comments
 (0)