-
Notifications
You must be signed in to change notification settings - Fork 18
feat: deployment dependency can also filter by versions #964
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -6,8 +6,9 @@ import ( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "go.opentelemetry.io/otel" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "go.opentelemetry.io/otel/attribute" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "workspace-engine/pkg/celutil" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "workspace-engine/pkg/oapi" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "workspace-engine/pkg/selector" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cel "workspace-engine/pkg/selector/langs/cel" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "workspace-engine/pkg/workspace/releasemanager/policy/evaluator" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "workspace-engine/pkg/workspace/releasemanager/policy/results" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -48,61 +49,6 @@ func (e *DeploymentDependencyEvaluator) Complexity() int { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return 3 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func (e *DeploymentDependencyEvaluator) findMatchingDeployments( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ctx context.Context, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| scope evaluator.EvaluatorScope, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) ([]*oapi.Deployment, error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| deploymentSelector := e.rule.DependsOn | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| deployments, err := e.getters.GetAllDeployments(ctx, scope.Environment.WorkspaceId) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return nil, fmt.Errorf("failed to get deployments: %w", err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| matchingDeployments := make([]*oapi.Deployment, 0) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for _, deployment := range deployments { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| matched, err := selector.Match(ctx, deploymentSelector, deployment) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return nil, fmt.Errorf("failed to match deployment selector: %w", err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if matched { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| matchingDeployments = append(matchingDeployments, deployment) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return matchingDeployments, nil | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func (e *DeploymentDependencyEvaluator) getUpstreamReleaseTargets( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ctx context.Context, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| matchingDeployments []*oapi.Deployment, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| resourceID string, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) []*oapi.ReleaseTarget { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| upstreamReleaseTargets := make([]*oapi.ReleaseTarget, 0, len(matchingDeployments)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| resourceTargets := e.getters.GetReleaseTargetsForResource(ctx, resourceID) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| deploymentToTargetMap := make(map[string]*oapi.ReleaseTarget) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for _, resourceTarget := range resourceTargets { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| deploymentToTargetMap[resourceTarget.DeploymentId] = resourceTarget | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for _, matchingDeployment := range matchingDeployments { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if target, ok := deploymentToTargetMap[matchingDeployment.Id]; ok { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| upstreamReleaseTargets = append(upstreamReleaseTargets, target) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return upstreamReleaseTargets | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func (e *DeploymentDependencyEvaluator) checkUpstreamTargetHasSuccessfulRelease( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| upstreamReleaseTarget *oapi.ReleaseTarget, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) bool { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| latestJob := e.getters.GetLatestCompletedJobForReleaseTarget(upstreamReleaseTarget) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if latestJob == nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return false | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return latestJob.Status == oapi.JobStatusSuccessful && latestJob.CompletedAt != nil | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func (e *DeploymentDependencyEvaluator) Evaluate( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ctx context.Context, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| scope evaluator.EvaluatorScope, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -117,55 +63,78 @@ func (e *DeploymentDependencyEvaluator) Evaluate( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| attribute.String("dependsOn", dependsOn), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| matchingDeployments, err := e.findMatchingDeployments(ctx, scope) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| program, err := cel.CompileProgram(dependsOn) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| span.RecordError(err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return results.NewDeniedResult( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fmt.Sprintf("Deployment dependency: failed to find matching deployments: %v", err), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fmt.Sprintf("Deployment dependency: failed to compile selector: %v", err), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| WithDetail("error", err.Error()). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| WithDetail("deployment_id", scope.Deployment.Id) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if len(matchingDeployments) == 0 { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return results.NewDeniedResult( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fmt.Sprintf( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "Deployment dependency: no matching deployments found for selector: %v", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| dependsOn, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| WithDetail("depends_on", dependsOn) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| upstreamReleaseTargets := e.getUpstreamReleaseTargets( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ctx, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| matchingDeployments, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| scope.Resource.Id, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if len(upstreamReleaseTargets) != cap(upstreamReleaseTargets) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| deployments, err := e.getters.GetAllDeployments(ctx, scope.Environment.WorkspaceId) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| span.RecordError(err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return results.NewDeniedResult( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fmt.Sprintf( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "Deployment dependency: some upstream release targets not found for resource: %v", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| scope.Resource.Id, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fmt.Sprintf("Deployment dependency: failed to get deployments: %v", err), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| WithDetail("depends_on", dependsOn) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| WithDetail("error", err.Error()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for _, upstreamReleaseTarget := range upstreamReleaseTargets { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if !e.checkUpstreamTargetHasSuccessfulRelease(upstreamReleaseTarget) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return results.NewDeniedResult( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| releaseTargets := e.getters.GetReleaseTargetsForResource(ctx, scope.Resource.Id) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| var evalErrors []string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for _, rt := range releaseTargets { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if rt.DeploymentId == scope.Deployment.Id && rt.EnvironmentId == scope.Environment.Id && | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| rt.ResourceId == scope.Resource.Id { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| continue | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| deployment := deployments[rt.DeploymentId] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if deployment == nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| continue | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+85
to
+98
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| releaseTargets := e.getters.GetReleaseTargetsForResource(ctx, scope.Resource.Id) | |
| for _, rt := range releaseTargets { | |
| deployment, err := e.getters.GetDeployment(ctx, rt.DeploymentId) | |
| if err != nil || deployment == nil { | |
| continue | |
| } | |
| releaseTargets := e.getters.GetReleaseTargetsForResource(ctx, scope.Resource.Id) | |
| deploymentsByID := make(map[string]*oapi.Deployment, len(releaseTargets)) | |
| for _, rt := range releaseTargets { | |
| if _, ok := deploymentsByID[rt.DeploymentId]; ok { | |
| continue | |
| } | |
| deployment, err := e.getters.GetDeployment(ctx, rt.DeploymentId) | |
| if err != nil || deployment == nil { | |
| continue | |
| } | |
| deploymentsByID[rt.DeploymentId] = deployment | |
| } | |
| for _, rt := range releaseTargets { | |
| deployment, ok := deploymentsByID[rt.DeploymentId] | |
| if !ok || deployment == nil { | |
| continue | |
| } |
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
Copilot
AI
Apr 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Errors from GetDeployment / GetCurrentlyDeployedVersion / CEL evaluation are silently ignored via continue. When the selector is invalid for the provided context (e.g., missing fields) or the DB call fails, the final denied message becomes misleading (“no upstream…matches”) and makes debugging difficult. Consider recording these errors (span/log) and/or returning a denied result that includes the first evaluation error and the release target key.
Uh oh!
There was an error while loading. Please reload this page.