Apply changes after review

pkg/engine/api/graphql/schema.graphql

@@ -335,7 +335,7 @@ input InterfacePolicyInput {
 
 input RulesForInterfaceInput {
   interface: ManifestReferenceInput!
-  oneOf:   [PolicyRuleInput!]!
+  oneOf: [PolicyRuleInput!]!
 }
 
 input PolicyRuleInput {
@@ -398,7 +398,7 @@ type InterfacePolicy {
 
 type RulesForInterface {
   interface: ManifestReferenceWithOptionalRevision!
-  oneOf:   [PolicyRule!]!
+  oneOf: [PolicyRule!]!
 }
 
 type PolicyRule {

pkg/engine/k8s/policy/metadata/metadata_resolver.go

@@ -10,7 +10,6 @@ import (
 	hublocalgraphql "capact.io/capact/pkg/hub/api/graphql/local"
 	hubpublicgraphql "capact.io/capact/pkg/hub/api/graphql/public"
 	"capact.io/capact/pkg/hub/client/public"
-	typesutil "capact.io/capact/pkg/hub/client/public/facade/types"
 	"capact.io/capact/pkg/sdk/apis/0.0.1/types"
 	multierr "github.com/hashicorp/go-multierror"
 
@@ -92,7 +91,7 @@ type TypeRefWithAdditionalRefs struct {
 
 func (r *Resolver) enrichWithParentNodes(ctx context.Context, refs map[string]hublocalgraphql.TypeInstanceTypeReference) (map[string]TypeRefWithAdditionalRefs, error) {
 	typesPath := r.mapToTypeRefs(refs)
-	gotAttachedTypes, err := typesutil.ListAdditionalRefs(ctx, r.hubCli, typesPath)
+	gotAttachedTypes, err := public.ListAdditionalRefs(ctx, r.hubCli, typesPath)
 	if err != nil {
 		return nil, errors.Wrap(err, "while fetching Types")
 	}
@@ -159,7 +158,6 @@ func (r *Resolver) setTypeRefsForAdditionalTypeInstances(policy *policy.Policy,
 	}
 }
 
-// TODO(storage) set also rule.TypeRef.Revision..
 func (r *Resolver) setTypeRefsForBackendTypeInstances(policy *policy.Policy, typeRefs map[string]TypeRefWithAdditionalRefs) {
 	for ruleIdx, rule := range policy.TypeInstance.Rules {
 		typeRef, exists := typeRefs[rule.Backend.ID]

pkg/hub/client/policy_enforced_client.go

@@ -113,7 +113,7 @@ func (e *PolicyEnforcedClient) ListTypeInstancesBackendsBasedOnPolicy(_ context.
 		out.SetByTypeRef(rule.TypeRef, rule.Backend)
 	}
 
-	// TODO(https://github.com/capactio/capact/issues/624):
+	// TODO(https://github.com/capactio/capact/issues/635):
 	// 2. Global defaults based on required TypeInstance injection
 	// e.mergedPolicy.Interface.Defaults
 

pkg/hub/client/public/facade/types/additional_refs.go → pkg/hub/client/public/additional_refs.go

@@ -1,23 +1,22 @@
-package types
+package public
 
 import (
 	"context"
 
 	"capact.io/capact/internal/ptr"
 	"capact.io/capact/internal/regexutil"
 	gqlpublicapi "capact.io/capact/pkg/hub/api/graphql/public"
-	"capact.io/capact/pkg/hub/client/public"
 	"capact.io/capact/pkg/sdk/apis/0.0.1/types"
 
 	"github.com/pkg/errors"
 )
 
 // listAdditionalRefsFields defines preset for response fields required for collection Type's spec.additionalRefs
-const listAdditionalRefsFields = public.TypeRevisionRootFields | public.TypeRevisionSpecAdditionalRefsField
+const listAdditionalRefsFields = TypeRevisionRootFields | TypeRevisionSpecAdditionalRefsField
 
 // ListAdditionalRefsClient defines external Hub calls used by ListAdditionalRefs.
 type ListAdditionalRefsClient interface {
-	ListTypes(ctx context.Context, opts ...public.TypeOption) ([]*gqlpublicapi.Type, error)
+	ListTypes(ctx context.Context, opts ...TypeOption) ([]*gqlpublicapi.Type, error)
 }
 
 // ListAdditionalRefsOutput holds Type's spec.additionalRefs entry indexed by TypeRef key.
@@ -30,9 +29,9 @@ type ListAdditionalRefsOutput map[types.TypeRef][]string
 func ListAdditionalRefs(ctx context.Context, cli ListAdditionalRefsClient, reqTypes []types.TypeRef) (ListAdditionalRefsOutput, error) {
 	filter := regexutil.OrStringSlice(mapToPaths(reqTypes))
 
-	opts := []public.TypeOption{
-		public.WithTypeRevisions(listAdditionalRefsFields),
-		public.WithTypeFilter(gqlpublicapi.TypeFilter{
+	opts := []TypeOption{
+		WithTypeRevisions(listAdditionalRefsFields),
+		WithTypeFilter(gqlpublicapi.TypeFilter{
 			PathPattern: ptr.String(filter),
 		}),
 	}

pkg/sdk/renderer/argo/dedicated_renderer.go

@@ -1040,29 +1040,31 @@ func (r *dedicatedRenderer) addOutputTypeInstancesToGraph(step *WorkflowStep, pr
 	return nil
 }
 
-func (*dedicatedRenderer) selectBackendAlias(upperStep, resolvedStep string) (string, error) {
+func (*dedicatedRenderer) selectBackendAlias(upperStep, resolvedStep string) (*string, error) {
 	if upperStep != "" && resolvedStep != "" {
-		return "", errors.Errorf("cannot override backend on capact-outputTypeInstances")
+		return nil, errors.Errorf("cannot override backend on capact-outputTypeInstances")
 	}
-	if upperStep != "" {
-		return upperStep, nil
+	for _, alias := range []string{upperStep, resolvedStep} {
+		if alias != "" {
+			return &alias, nil
+		}
 	}
-	return resolvedStep, nil
+	return nil, nil
 }
 
-func (*dedicatedRenderer) selectBackend(alias string, typeRef types.TypeRef, backends policy.TypeInstanceBackendCollection) (policy.TypeInstanceBackend, error) {
-	if alias == "" { // alias not set, get the Policy default based on TypeRef
+func (*dedicatedRenderer) selectBackend(alias *string, typeRef types.TypeRef, backends policy.TypeInstanceBackendCollection) (policy.TypeInstanceBackend, error) {
+	if alias == nil { // alias not set, get the Policy default based on TypeRef
 		backend, _ := backends.GetByTypeRef(typeRef)
 		return backend, nil
 	}
 
 	// when alias is specified, required TypeInstance needs to be injected and be of Hub storage type
-	backend, found := backends.GetByAlias(alias)
+	backend, found := backends.GetByAlias(*alias)
 	if !found {
-		return policy.TypeInstanceBackend{}, fmt.Errorf("cannot find backend storage for specified %s alias", alias)
+		return policy.TypeInstanceBackend{}, fmt.Errorf("cannot find backend storage for specified %q alias", *alias)
 	}
 	if !backend.ExtendsHubStorage {
-		return policy.TypeInstanceBackend{}, fmt.Errorf("TypeInstance with %q alias is not a Hub storage", alias)
+		return policy.TypeInstanceBackend{}, fmt.Errorf("TypeInstance with %q alias is not a Hub storage", *alias)
 	}
 
 	return backend, nil

pkg/sdk/validation/manifest/json_remote_implementation.go

@@ -8,7 +8,6 @@ import (
 
 	gqlpublicapi "capact.io/capact/pkg/hub/api/graphql/public"
 	"capact.io/capact/pkg/hub/client/public"
-	typesutil "capact.io/capact/pkg/hub/client/public/facade/types"
 	"capact.io/capact/pkg/sdk/apis/0.0.1/types"
 	wfv1 "github.com/argoproj/argo-workflows/v3/pkg/apis/workflow/v1alpha1"
 
@@ -281,7 +280,7 @@ func (v *RemoteImplementationValidator) checkParentNodesAssociation(ctx context.
 
 	var validationErrs []error
 	for parentNode, expTypesRefs := range relations {
-		gotAttachedTypes, err := typesutil.ListAdditionalRefs(ctx, v.hub, expTypesRefs)
+		gotAttachedTypes, err := public.ListAdditionalRefs(ctx, v.hub, expTypesRefs)
 		if err != nil {
 			return ValidationResult{}, errors.Wrap(err, "while fetching Types based on parent node")
 		}
@@ -301,7 +300,7 @@ func (v *RemoteImplementationValidator) checkParentNodesAssociation(ctx context.
 	return ValidationResult{Errors: validationErrs}, nil
 }
 
-func (v *RemoteImplementationValidator) detectMissingChildren(gotAttachedTypes typesutil.ListAdditionalRefsOutput, expAttachedTypes []types.TypeRef, expParent string) []string {
+func (v *RemoteImplementationValidator) detectMissingChildren(gotAttachedTypes public.ListAdditionalRefsOutput, expAttachedTypes []types.TypeRef, expParent string) []string {
 	var missingChildren []string
 
 	for _, exp := range expAttachedTypes {

pkg/sdk/validation/policy/policy.go

@@ -95,37 +95,14 @@ func (v *Validator) ValidateAdditionalInputParameters(ctx context.Context, param
 
 // ValidateTypeInstancesMetadata validates that every TypeInstance has metadata resolved.
 func (v *Validator) ValidateTypeInstancesMetadata(in policy.Policy) validation.Result {
-	resultBldr := validation.NewResultBuilder("Metadata for")
-
-	unresolvedIDs := map[string]struct{}{}
 	unresolvedTypeInstances := metadata.TypeInstanceIDsWithUnresolvedMetadataForPolicy(in)
-	for _, ti := range unresolvedTypeInstances {
-		resultBldr.ReportIssue(string(ti.Kind), "missing Type reference for %s", ti.String(false))
-		unresolvedIDs[ti.ID] = struct{}{}
-	}
-
-	for _, rule := range in.TypeInstance.Rules {
-		if _, unresolved := unresolvedIDs[rule.Backend.ID]; unresolved {
-			continue // Type was not resolved, so `ExtendsHubStorage` has zero value
-		}
-
-		if rule.Backend.ExtendsHubStorage {
-			continue
-		}
-		ref := metadata.TypeInstanceMetadata{
-			ID:          rule.Backend.ID,
-			Description: rule.Backend.Description,
-		}
-		resultBldr.ReportIssue("BackendTypeInstance", "Type reference %s is not a Hub storage", ref.String(false))
-	}
-
-	return resultBldr.Result()
+	return v.validationResultForTIMetadata(unresolvedTypeInstances, in.TypeInstance.Rules)
 }
 
 // ValidateTypeInstancesMetadataForRule validates whether the TypeInstance injection metadata are resolved.
 func (v *Validator) ValidateTypeInstancesMetadataForRule(in policy.Rule) validation.Result {
 	unresolvedTypeInstances := metadata.TypeInstanceIDsWithUnresolvedMetadataForRule(in)
-	return v.validationResultForTIMetadata(unresolvedTypeInstances)
+	return v.validationResultForTIMetadata(unresolvedTypeInstances, nil)
 }
 
 // AreTypeInstancesMetadataResolved returns whether every TypeInstance has metadata resolved.
@@ -142,15 +119,28 @@ func (v *Validator) hasImplAdditionalInputParams(impl gqlpublicapi.Implementatio
 	return true
 }
 
-func (v *Validator) validationResultForTIMetadata(tis []metadata.TypeInstanceMetadata) validation.Result {
-	if len(tis) == 0 {
-		return validation.Result{}
-	}
-
+func (v *Validator) validationResultForTIMetadata(tis []metadata.TypeInstanceMetadata, rules []policy.RulesForTypeInstance) validation.Result {
 	resultBldr := validation.NewResultBuilder("Metadata for")
 
+	unresolvedIDs := map[string]struct{}{}
 	for _, ti := range tis {
 		resultBldr.ReportIssue(string(ti.Kind), "missing Type reference for %s", ti.String(false))
+		unresolvedIDs[ti.ID] = struct{}{}
+	}
+
+	for _, rule := range rules {
+		if _, unresolved := unresolvedIDs[rule.Backend.ID]; unresolved {
+			continue // Type was not resolved, so `ExtendsHubStorage` has zero value
+		}
+
+		if rule.Backend.ExtendsHubStorage {
+			continue
+		}
+		ref := metadata.TypeInstanceMetadata{
+			ID:          rule.Backend.ID,
+			Description: rule.Backend.Description,
+		}
+		resultBldr.ReportIssue("BackendTypeInstance", "Type reference %s is not a Hub storage", ref.String(false))
 	}
 
 	return resultBldr.Result()

test/e2e/action_test.go

@@ -28,10 +28,10 @@ import (
 )
 
 const (
-	actionPassingPath      = "cap.interface.capactio.capact.validation.action.passing"
-	uploadTypePath         = "cap.type.capactio.capact.validation.upload"
-	builtinStorageTypePath = "cap.core.type.hub.storage.neo4j"
-	singleKeyTypePath      = "cap.type.capactio.capact.validation.single-key"
+	actionPassingInterfacePath = "cap.interface.capactio.capact.validation.action.passing"
+	uploadTypePath             = "cap.type.capactio.capact.validation.upload"
+	builtinStorageTypePath     = "cap.core.type.hub.storage.neo4j"
+	singleKeyTypePath          = "cap.type.capactio.capact.validation.single-key"
 )
 
 func getActionName() string {
@@ -61,7 +61,6 @@ var _ = Describe("Action", func() {
 		engineClient.DeleteAction(ctx, actionName)
 		engineClient.DeleteAction(ctx, failingActionName)
 	})
-	const actionPath = "cap.interface.capactio.capact.validation.action.passing"
 
 	Context("Action execution", func() {
 
@@ -83,7 +82,7 @@ var _ = Describe("Action", func() {
 			defer updateTICleanup()
 
 			By("1.3 Creating TypeInstance that describes Helm storage")
-			helmStorage := getTypeInstanceHelmStorage()
+			helmStorage := fixHelmStorageTypeInstanceCreateInput()
 			helmStorageTI, helmStorageTICleanup := createTypeInstance(ctx, hubClient, helmStorage)
 			defer helmStorageTICleanup()
 
@@ -99,7 +98,7 @@ var _ = Describe("Action", func() {
 
 			By("2. Expecting Implementation A is picked and builtin storage is used...")
 
-			action := createActionAndWaitForReadyToRunPhase(ctx, engineClient, actionName, actionPath, inputData)
+			action := createActionAndWaitForReadyToRunPhase(ctx, engineClient, actionName, actionPassingInterfacePath, inputData)
 			assertActionRenderedWorkflowContains(action, "echo '%s'", implIndicatorValue)
 			runActionAndWaitForSucceeded(ctx, engineClient, actionName)
 
@@ -127,7 +126,7 @@ var _ = Describe("Action", func() {
 			setGlobalTestPolicy(ctx, engineClient, withHelmBackendForUploadTypeRef(helmStorageTI.ID))
 
 			By("7. Expecting Implementation A is picked and the Helm storage is used for uploaded TypeInstance...")
-			action = createActionAndWaitForReadyToRunPhase(ctx, engineClient, actionName, actionPath, inputData)
+			action = createActionAndWaitForReadyToRunPhase(ctx, engineClient, actionName, actionPassingInterfacePath, inputData)
 			assertActionRenderedWorkflowContains(action, "echo '%s'", implIndicatorValue)
 			runActionAndWaitForSucceeded(ctx, engineClient, actionName)
 
@@ -159,7 +158,7 @@ var _ = Describe("Action", func() {
 			defer updateTICleanup()
 
 			By("1.3 Creating TypeInstance that describes Helm storage")
-			helmStorage := getTypeInstanceHelmStorage()
+			helmStorage := fixHelmStorageTypeInstanceCreateInput()
 			helmStorageTI, helmStorageTICleanup := createTypeInstance(ctx, hubClient, helmStorage)
 			defer helmStorageTICleanup()
 
@@ -189,7 +188,7 @@ var _ = Describe("Action", func() {
 			setGlobalTestPolicy(ctx, engineClient, prependInjectRuleForPassingActionInterface(globalPolicyRequiredTypeInstances))
 
 			By("3. Expecting Implementation B is picked and injected Helm storage is used...")
-			action := createActionAndWaitForReadyToRunPhase(ctx, engineClient, actionName, actionPath, inputData)
+			action := createActionAndWaitForReadyToRunPhase(ctx, engineClient, actionName, actionPassingInterfacePath, inputData)
 			assertActionRenderedWorkflowContains(action, "echo '%s'", implIndicatorValue)
 			runActionAndWaitForSucceeded(ctx, engineClient, actionName)
 
@@ -395,7 +394,7 @@ func getTypeInstanceInputForUpdate() *hublocalgraphql.CreateTypeInstanceInput {
 	}
 }
 
-func getTypeInstanceHelmStorage() *hublocalgraphql.CreateTypeInstanceInput {
+func fixHelmStorageTypeInstanceCreateInput() *hublocalgraphql.CreateTypeInstanceInput {
 	return &hublocalgraphql.CreateTypeInstanceInput{
 		TypeRef: &hublocalgraphql.TypeInstanceTypeReferenceInput{
 			Path:     "cap.type.helm.storage",
@@ -530,7 +529,7 @@ func prependInjectRuleForPassingActionInterface(reqInput []*enginegraphql.Requir
 	}
 	return func(policy *enginegraphql.PolicyInput) {
 		for idx, rule := range policy.Interface.Rules {
-			if rule.Interface.Path != actionPassingPath {
+			if rule.Interface.Path != actionPassingInterfacePath {
 				continue
 			}
 			policy.Interface.Rules[idx].OneOf = append([]*enginegraphql.PolicyRuleInput{
@@ -656,7 +655,7 @@ func fixGQLTestPolicyInput() *enginegraphql.PolicyInput {
 		Interface: &enginegraphql.InterfacePolicyInput{
 			Rules: []*enginegraphql.RulesForInterfaceInput{
 				{
-					Interface: manifestRef(actionPassingPath),
+					Interface: manifestRef(actionPassingInterfacePath),
 					OneOf: []*enginegraphql.PolicyRuleInput{
 						{
 							ImplementationConstraints: &enginegraphql.PolicyRuleImplementationConstraintsInput{