fix(artifacts): Ignoring the artifacts filter for pipeline stage

[nemesisOsorio]

fixes spinnaker/#6850

There are two main ways to trigger a pipeline
using a trigger and using the Pipeline Stage

Seems like the field expectedArtifacts has 2 use cases:

1. Artifact Constraints in Automated Triggers
2. Use the artifact in a stage in the pipeline(e.g. Deploy (Manifest) in Required Artifacts to Bind )

Triggering a Pipeline using the Pipeline Stage or the Trigger Type Pipeline are using the same mechanism,
the only way to know if the trigger comes from the Pipeline Stage is if the trigger has the field parentPipelineStageId

If we always filter the expectedArtifactIds the ArtifactUtils is not populating the field resolvedExpectedArtifacts which could be used later for example in ResolveDeploySourceManifestTask

[dbyron-sf]

spinnaker/spinnaker#6850 seems like it more about the artifacts that orca provides when it triggers a pipeline...As in, if pipeline B has a trigger on the completion of pipeline A, what artifacts does orca provide in the trigger of pipeline B when A completes?

[nemesisOsorio]

I updated the code comments and added details to the PR

[dbyron-sf]

I still don't understand this. If orca provides artifacts when it triggers a pipeline, and a trigger has artifact constraints, wouldn't we want to honor those constraints? If it breaks backwards compatibility to do that, then let's add a feature flag that defaults to the old behavior for now.

[dbyron-sf]

Potentially related, what would it take to add a test that's more at the level of spinnaker/spinnaker#6850? Like, one pipeline triggers another and we get to verify that it actually triggers?

[xibz]

@nemesisOsorio I am also really confused to why we would ever want to do this. I tried reading the description, but it still isn't clear what the use case for this is. Is this due to child pipelines not understanding what is coming from the parent? Just blatantly ignoring expected artifacts seems the wrong way to go about it.

What are we trying to achieve exactly?

[nemesisOsorio]

the following is a pipeline example, it has expectedArtifacts without triggers, the expected artifact (docker-artifact) is used in Deploy (Manifest) stage, and the pipeline could be executed using a parent pipeline with Pipeline Stage, this is the scenario from spinnaker/spinnaker#6850

{
    "appConfig": {},
    "expectedArtifacts": [
        {
            "defaultArtifact": {
                "customKind": true,
                "id": "5ef41d5e-b693-4cef-9ef8-483df68d06f5"
            },
            "displayName": "docker-artifact",
            "id": "1661108f-bb56-43d8-b57b-961b10baafd4",
            "matchArtifact": {
                "artifactAccount": "docker-registry",
                "id": "5d6ae9b8-37cc-4b4d-b458-a44d310daf43",
                "name": "docker.io/my-org/my-image",
                "type": "docker/image"
            },
            "useDefaultArtifact": false,
            "usePriorArtifact": false
        }
    ],
    "keepWaitingPipelines": false,
    "lastModifiedBy": "anonymous",
    "limitConcurrent": true,
    "schema": "1",
    "spelEvaluator": "v4",
    "stages": [
        {
            "expectedArtifacts": [
                {
                    "defaultArtifact": {
                        "customKind": true,
                        "id": "ffbaa238-2587-48e8-bf53-b31546787000"
                    },
                    "displayName": "angry-hound-58",
                    "id": "ac486336-453c-4353-a3a6-92d312ca54c6",
                    "matchArtifact": {
                        "artifactAccount": "embedded-artifact",
                        "customKind": false,
                        "id": "c2c797e8-4ba3-450d-9825-91af25bfca6a",
                        "type": "embedded/base64"
                    },
                    "useDefaultArtifact": false,
                    "usePriorArtifact": false
                }
            ],
            "inputArtifact": {
                "account": "gitrepo",
                "artifact": {
                    "artifactAccount": "gitrepo",
                    "customKind": true,
                    "id": "f1b2688d-34fa-4f9a-973e-03e5a2d130b3",
                    "location": "",
                    "reference": "https://github.com/my-org/my-kustomize-repo.git",
                    "type": "git/repo",
                    "version": "main"
                }
            },
            "inputArtifacts": null,
            "kustomizeFilePath": "nginx/kustomization.yml",
            "name": "Bake (Manifest)",
            "overrides": {},
            "refId": "1",
            "requisiteStageRefIds": [],
            "templateRenderer": "KUSTOMIZE",
            "type": "bakeManifest"
        },
        {
            "account": "k8s-splat-dev",
            "cloudProvider": "kubernetes",
            "manifestArtifactId": "ac486336-453c-4353-a3a6-92d312ca54c6",
            "moniker": {
                "app": "artifact-error"
            },
            "name": "Deploy (Manifest)",
            "namespaceOverride": "nemesis-dev",
            "refId": "2",
            "requiredArtifactIds": [
                "1661108f-bb56-43d8-b57b-961b10baafd4"
            ],
            "requiredArtifacts": [],
            "requisiteStageRefIds": [
                "1"
            ],
            "skipExpressionEvaluation": true,
            "source": "artifact",
            "trafficManagement": {
                "enabled": false,
                "options": {
                    "enableTraffic": false,
                    "services": []
                }
            },
            "type": "deployManifest"
        }
    ],
    "triggers": [],
    "updateTs": "1689781854000"
}

I see what you mean, I'll try to create a fix only for Deploy (Manifest) stage, or create the feature flag, Thank you

[nemesisOsorio]

btw: if the pipeline is executed using an Automated trigger (even a pipeline trigger) my changes are still constraining the artifacts

[xibz]

this is the scenario from spinnaker/spinnaker#6850

Hmm, I am not too sure why they highlighted the Skip SpEL since that doesn't pertain to expected artifacts. I think there's some confusion on what the Skip SpEL does for a lot of people. It took me a little bit to figure out as you'd think it'd skip SpEL.

So from this https://spinnaker.io/changelogs/1.30.0-changelog/#artifact-handling, the biggest thing is it is fixing the unexpected behavior, and it sounds like people were relying on that old bugged behavior.

I think your suggestion is the correct course of action, useDefaultArtifact. However, we still need to be careful with breaking changes.

From the changelog

If any of them are missing, the pipeline will not trigger.

If you’ve relied on this bug, you’ll need to add manually add all the artifact constraints to all triggers to replicate the previous behavior.

So it does look intentional to be a breaking change.

What does that mean what we want to do? Do we want to add a feature that relies on the bug, or be more opinionated? @dbyron-sf @nemesisOsorio

[nemesisOsorio]

Closing in favor of #4489
I believe that is a better approach
@xibz @dbyron-sf can you please take a look?