Skip to content
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

Jump to bottom

fix(expressions): fetch labels from actually deployed manfiest #4508

Merged
merged 3 commits into from
Aug 24, 2023
Merged

fix(expressions): fetch labels from actually deployed manfiest #4508

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
940fa8e
fix(expressions): fetch labels from actually deployed manfiest
mattgogerly Aug 18, 2023
94b1790
fix(expressions): handle dot in label key
mattgogerly Aug 18, 2023
f543312
Merge branch 'master' into fix-manifest-label-value-expression
mergify[bot] Aug 24, 2023
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
6 changes: 5 additions & 1 deletion ...ker/orca/pipeline/expressions/functions/ManifestLabelValueExpressionFunctionProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,11 @@ public static String manifestLabelValue(
"A valid Deploy Manifest stage name is required for this function");
}

List<Map> manifests = (List<Map>) stage.get().getContext().get("manifests");
// using outputs.manifests to give access to labels added by Spinnaker itself during the
// deployment
// this is safe as we assert above that we're only using successful deploy stages, so this key
// should always exist
List<Map> manifests = (List<Map>) stage.get().getContext().get("outputs.manifests");
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this safe timing-wise? Like, could this run when manifests exists, but output.manifests doesn't?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm reasonably confident outputs.manifests should always exist once a deploy stage is succeeded - and this expression only considers stages in a succeeded state.

Copy link
Contributor

Choose a reason for hiding this comment

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

Aaah, I see it a few lines up.


if (manifests == null || manifests.size() == 0) {
throw new SpelHelperFunctionException(
Expand Down
23 changes: 20 additions & 3 deletions ...ca/pipeline/expressions/functions/ManifestLabelValueExpressionFunctionProviderSpec.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,22 @@ class ManifestLabelValueExpressionFunctionProviderSpec extends Specification {
]
]
]
],
"outputs.manifests": [
[
"kind": "ReplicaSet",
"spec": [
"template": [
"metadata": [
"labels": [
"my-label-key": "my-label-value",
"my-other-label-key": "my-other-label-value",
"moniker.spinnaker.io/sequence": "0"
]
]
]
]
]
]
)
status = SUCCEEDED
Expand All @@ -60,9 +76,10 @@ class ManifestLabelValueExpressionFunctionProviderSpec extends Specification {
manifestLabelValue(deployManifestPipeline, "Deploy ReplicaSet", "ReplicaSet", labelKey) == expectedLabelValue

where:
labelKey || expectedLabelValue
"my-label-key" || "my-label-value"
"my-other-label-key" || "my-other-label-value"
labelKey || expectedLabelValue
"my-label-key" || "my-label-value"
"my-other-label-key" || "my-other-label-value"
"moniker.spinnaker.io/sequence" || "0"
}

def "manifestLabelValue should raise exception if stage, manifest, or label not found"() {
Expand Down
Loading