Skip to content

feat: add status to the ModelValidation CRs, tracking pod and modelva… #47

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: add status to the ModelValidation CRs, tracking pod and modelva… #47

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 78 additions & 0 deletions api/v1alpha1/modelvalidation_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ limitations under the License.
package v1alpha1

import (
"crypto/sha256"
"fmt"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

Expand Down Expand Up @@ -69,15 +72,54 @@ type ModelValidationSpec struct {
Config ValidationConfig `json:"config"`
}

// PodTrackingInfo contains information about a tracked pod
type PodTrackingInfo struct {
// Name is the name of the pod
Name string `json:"name"`
// UID is the unique identifier of the pod
UID string `json:"uid"`
// InjectedAt is when the pod was injected
InjectedAt metav1.Time `json:"injectedAt"`
}

// ModelValidationStatus defines the observed state of ModelValidation
type ModelValidationStatus struct {
// INSERT ADDITIONAL STATUS FIELD - define observed state of cluster
// Important: Run "make" to regenerate code after modifying this file
Conditions []metav1.Condition `json:"conditions,omitempty"`

// InjectedPodCount is the number of pods that have been injected with validation
InjectedPodCount int32 `json:"injectedPodCount"`

// UninjectedPodCount is the number of pods that have the label but were not injected
UninjectedPodCount int32 `json:"uninjectedPodCount"`

// OrphanedPodCount is the number of injected pods that reference this CR but are inconsistent
OrphanedPodCount int32 `json:"orphanedPodCount"`

// AuthMethod indicates which authentication method is being used
AuthMethod string `json:"authMethod,omitempty"`

// InjectedPods contains detailed information about injected pods
InjectedPods []PodTrackingInfo `json:"injectedPods,omitempty"`

// UninjectedPods contains detailed information about pods that should have been injected but weren't
UninjectedPods []PodTrackingInfo `json:"uninjectedPods,omitempty"`

// OrphanedPods contains detailed information about pods that are injected but inconsistent
OrphanedPods []PodTrackingInfo `json:"orphanedPods,omitempty"`

// LastUpdated is the timestamp of the last status update
LastUpdated metav1.Time `json:"lastUpdated,omitempty"`
}

// +kubebuilder:object:root=true
// +kubebuilder:subresource:status
// +kubebuilder:printcolumn:name="Auth Method",type=string,JSONPath=`.status.authMethod`
// +kubebuilder:printcolumn:name="Injected Pods",type=integer,JSONPath=`.status.injectedPodCount`
// +kubebuilder:printcolumn:name="Uninjected Pods",type=integer,JSONPath=`.status.uninjectedPodCount`
// +kubebuilder:printcolumn:name="Orphaned Pods",type=integer,JSONPath=`.status.orphanedPodCount`
// +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp"

// ModelValidation is the Schema for the modelvalidations API
type ModelValidation struct {
Expand All @@ -97,6 +139,42 @@ type ModelValidationList struct {
Items []ModelValidation `json:"items"`
}

// GetAuthMethod returns the authentication method being used
func (mv *ModelValidation) GetAuthMethod() string {
if mv.Spec.Config.SigstoreConfig != nil {
return "sigstore"
} else if mv.Spec.Config.PkiConfig != nil {
return "pki"
} else if mv.Spec.Config.PrivateKeyConfig != nil {
return "private-key"
}
return "unknown"
}

// GetConfigHash returns a hash of the validation configuration for drift detection
func (mv *ModelValidation) GetConfigHash() string {
Copy link

@osmman osmman Aug 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, instead of creating a custom hashing method for drift detection, it's best to use Kubernetes build-in fields. These are more robust and are the standard pattern for operators.

Use .metadata.generation and a status field like status.observedGeneration or ObservedGeneration field within a Condition struct. The reconciliation loop should follow this logic:

  1. On each reconcile, compare .metadata.generation with status.observedGeneration.
  2. If the values don't match, it indicates the spec has been updated and a reconciliation is needed.
  3. After a successful reconciliation, update status.observedGeneration to equal .metadata.generation.

Using .metadata.resourceVersion is another option, but generation is typically preferred for tracking spec-only changes, as resourceVersion changes on every update, including status updates.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hashing is not just about detecting drift, it's also used for tracking within PodInfo so I know which specific configuration applied to the pods. I have no problem adding the observed generation in addition to the hash, as a quick sanity check, but not replacing the hash. I wouldn't want to use resourceVersion, as that comes from etcd and represents every change to the resource.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@osmman I've updated the code to add the generation check, so we don't need to redo the hash if that hasn't changed. I also tidied up some older code, it was still tracking resourceVersion but I never used it.

Please take a look at the changes

return mv.Spec.Config.GetConfigHash()
}

// GetConfigHash returns a hash of the validation configuration for drift detection
func (vc *ValidationConfig) GetConfigHash() string {
hasher := sha256.New()

if vc.SigstoreConfig != nil {
hasher.Write([]byte("sigstore"))
hasher.Write([]byte(vc.SigstoreConfig.CertificateIdentity))
hasher.Write([]byte(vc.SigstoreConfig.CertificateOidcIssuer))
} else if vc.PkiConfig != nil {
hasher.Write([]byte("pki"))
hasher.Write([]byte(vc.PkiConfig.CertificateAuthority))
} else if vc.PrivateKeyConfig != nil {
hasher.Write([]byte("privatekey"))
hasher.Write([]byte(vc.PrivateKeyConfig.KeyPath))
}

return fmt.Sprintf("%x", hasher.Sum(nil))[:16] // Use first 16 chars for brevity
}

func init() {
SchemeBuilder.Register(&ModelValidation{}, &ModelValidationList{})
}
38 changes: 38 additions & 0 deletions api/v1alpha1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

65 changes: 65 additions & 0 deletions cmd/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,11 @@ import (
"flag"
"os"
"path/filepath"
"time"

"github.com/sigstore/model-validation-operator/internal/constants"
"github.com/sigstore/model-validation-operator/internal/controller"
"github.com/sigstore/model-validation-operator/internal/tracker"
"github.com/sigstore/model-validation-operator/internal/utils"
"github.com/sigstore/model-validation-operator/internal/webhooks"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
Expand All @@ -47,6 +50,16 @@ import (
// +kubebuilder:scaffold:imports
)

const (
// Default configuration values for the status tracker
defaultDebounceDuration = 500 * time.Millisecond
defaultRetryBaseDelay = 100 * time.Millisecond
defaultRetryMaxDelay = 16 * time.Second
defaultRateLimitQPS = 10.0
defaultRateLimitBurst = 100
defaultStatusUpdateTimeout = 30 * time.Second
)

var (
scheme = runtime.NewScheme()
setupLog = ctrl.Log.WithName("setup")
Expand All @@ -69,6 +82,14 @@ func main() {
var secureMetrics bool
var enableHTTP2 bool
var tlsOpts []func(*tls.Config)

// Status tracker configuration
var debounceDuration time.Duration
var retryBaseDelay time.Duration
var retryMaxDelay time.Duration
var rateLimitQPS float64
var rateLimitBurst int
var statusUpdateTimeout time.Duration
flag.StringVar(&metricsAddr, "metrics-bind-address", "0", "The address the metrics endpoint binds to. "+
"Use :8443 for HTTPS or :8080 for HTTP, or leave as 0 to disable the metrics service.")
flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.")
Expand All @@ -91,6 +112,20 @@ func main() {
"MODEL_TRANSPARENCY_CLI_IMAGE",
constants.ModelTransparencyCliImage,
"Model transparency CLI image to be used.")

// Status tracker configuration flags
flag.DurationVar(&debounceDuration, "debounce-duration", defaultDebounceDuration,
"Time to wait for more changes before updating status")
flag.DurationVar(&retryBaseDelay, "retry-base-delay", defaultRetryBaseDelay,
"Base delay for exponential backoff retries")
flag.DurationVar(&retryMaxDelay, "retry-max-delay", defaultRetryMaxDelay,
"Maximum delay for exponential backoff retries")
flag.Float64Var(&rateLimitQPS, "rate-limit-qps", defaultRateLimitQPS,
"Overall rate limit for status updates (queries per second)")
flag.IntVar(&rateLimitBurst, "rate-limit-burst", defaultRateLimitBurst,
"Burst capacity for overall rate limit")
flag.DurationVar(&statusUpdateTimeout, "status-update-timeout", defaultStatusUpdateTimeout,
"Timeout for status update operations")
opts := zap.Options{
Development: true,
}
Expand Down Expand Up @@ -246,6 +281,36 @@ func main() {
Handler: interceptor,
})

statusTracker := tracker.NewStatusTracker(mgr.GetClient(), tracker.StatusTrackerConfig{
DebounceDuration: debounceDuration,
RetryBaseDelay: retryBaseDelay,
RetryMaxDelay: retryMaxDelay,
RateLimitQPS: rateLimitQPS,
RateLimitBurst: rateLimitBurst,
StatusUpdateTimeout: statusUpdateTimeout,
})
defer statusTracker.Stop()

podReconciler := &controller.PodReconciler{
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
Tracker: statusTracker,
}
if err := podReconciler.SetupWithManager(mgr); err != nil {
setupLog.Error(err, "unable to create pod controller")
os.Exit(1)
}

mvReconciler := &controller.ModelValidationReconciler{
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
Tracker: statusTracker,
}
if err := mvReconciler.SetupWithManager(mgr); err != nil {
setupLog.Error(err, "unable to create ModelValidation controller")
os.Exit(1)
}

setupLog.Info("starting manager")
if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil {
setupLog.Error(err, "problem running manager")
Expand Down
114 changes: 113 additions & 1 deletion config/crd/bases/ml.sigstore.dev_modelvalidations.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,23 @@ spec:
singular: modelvalidation
scope: Namespaced
versions:
- name: v1alpha1
- additionalPrinterColumns:
- jsonPath: .status.authMethod
name: Auth Method
type: string
- jsonPath: .status.injectedPodCount
name: Injected Pods
type: integer
- jsonPath: .status.uninjectedPodCount
name: Uninjected Pods
type: integer
- jsonPath: .status.orphanedPodCount
name: Orphaned Pods
type: integer
- jsonPath: .metadata.creationTimestamp
name: Age
type: date
name: v1alpha1
schema:
openAPIV3Schema:
description: ModelValidation is the Schema for the modelvalidations API
Expand Down Expand Up @@ -89,6 +105,10 @@ spec:
status:
description: ModelValidationStatus defines the observed state of ModelValidation
properties:
authMethod:
description: AuthMethod indicates which authentication method is being
used
type: string
conditions:
description: |-
INSERT ADDITIONAL STATUS FIELD - define observed state of cluster
Expand Down Expand Up @@ -148,6 +168,98 @@ spec:
- type
type: object
type: array
injectedPodCount:
description: InjectedPodCount is the number of pods that have been
injected with validation
format: int32
type: integer
injectedPods:
description: InjectedPods contains detailed information about injected
pods
items:
description: PodTrackingInfo contains information about a tracked
pod
properties:
injectedAt:
description: InjectedAt is when the pod was injected
format: date-time
type: string
name:
description: Name is the name of the pod
type: string
uid:
description: UID is the unique identifier of the pod
type: string
required:
- injectedAt
- name
- uid
type: object
type: array
lastUpdated:
description: LastUpdated is the timestamp of the last status update
format: date-time
type: string
orphanedPodCount:
description: OrphanedPodCount is the number of injected pods that
reference this CR but are inconsistent
format: int32
type: integer
orphanedPods:
description: OrphanedPods contains detailed information about pods
that are injected but inconsistent
items:
description: PodTrackingInfo contains information about a tracked
pod
properties:
injectedAt:
description: InjectedAt is when the pod was injected
format: date-time
type: string
name:
description: Name is the name of the pod
type: string
uid:
description: UID is the unique identifier of the pod
type: string
required:
- injectedAt
- name
- uid
type: object
type: array
uninjectedPodCount:
description: UninjectedPodCount is the number of pods that have the
label but were not injected
format: int32
type: integer
uninjectedPods:
description: UninjectedPods contains detailed information about pods
that should have been injected but weren't
items:
description: PodTrackingInfo contains information about a tracked
pod
properties:
injectedAt:
description: InjectedAt is when the pod was injected
format: date-time
type: string
name:
description: Name is the name of the pod
type: string
uid:
description: UID is the unique identifier of the pod
type: string
required:
- injectedAt
- name
- uid
type: object
type: array
required:
- injectedPodCount
- orphanedPodCount
- uninjectedPodCount
type: object
type: object
served: true
Expand Down
Loading