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.

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

Trigger's default parameter value not being applied #1238

Closed
atombender opened this issue Jun 4, 2021 · 6 comments · Fixed by #1254
Closed

Trigger's default parameter value not being applied #1238

atombender opened this issue Jun 4, 2021 · 6 comments · Fixed by #1254
Labels

Comments

@atombender
Copy link

atombender commented Jun 4, 2021

I have a sensor with two dependencies in an ||. When one dependency triggers, I want to ensure that the other dependency's parameter has a default value.

Here's my sensor, simplified:

apiVersion: argoproj.io/v1alpha1
kind: Sensor
metadata:
  name: github
spec:
  dependencies:
  - name: dep1
    eventSourceName: github
    eventName: dep1
    filters:
      data:
      - {path: body.check_suite.app.external_url, type: string, comparator: "=", value: ["https://circleci.com"]}
      - {path: body.check_suite.conclusion, type: string, comparator: "=", value: [success]}
  - name: dep2
    eventSourceName: github
    eventName: dep2
    filters:
      data:
      - {path: body.check_suite.app.external_url, type: string, comparator: "=", value: ["https://circleci.com"]}
      - {path: body.check_suite.conclusion, type: string, comparator: "=", value: [success]}

  triggers:
  - template:
      name: argo-workflow-trigger
      conditions: "dep1 || dep2"
      k8s:
        group: argoproj.io
        version: v1alpha1
        resource: workflows
        operation: create

        parameters:
        - src:
            dependencyName: dep1
            dataKey: body.check_suite.head_sha
            value: main
          dest: spec.arguments.parameters.0.value
        - src:
            dependencyName: dep2
            dataKey: body.check_suite.head_sha
            value: main
          dest: spec.arguments.parameters.1.value

        source:
          resource:
            apiVersion: argoproj.io/v1alpha1
            kind: Workflow
            metadata:
              labels:
                workflows.argoproj.io/archive-strategy: "false"
            spec:
              entrypoint: main
              arguments:
                parameters:
                - name: dep1_commit
                  value: main
                - name: dep2_commit
                  value: main
              workflowTemplateRef:
                name: test

If the sensor triggers on dep1 or dep2, the dataKey is successfully extracted and stored in the right workflow parameter. However, the other parameter becomes empty, despite the fact that there is a value set on the src.

As you can see, I also tried providing a default value in the workflow's parameters, but the value still becomes empty.

Looking at the documentation, there's a discrepancy between the API docs and the guide. The API docs say (my emphasis):

Value is the default literal value to use for this parameter source This is only used if the DataKey is invalid. If the DataKey is invalid and this is not defined, this param source will produce an error.

The parameterization tutorial, on the other hand, suggests that value is used as a default value if the dataKey data is missing.

There's nothing in the sensor log about failing to read the event data, even though the code (from what I can read here) should complain.

This leads me to think that maybe the dependency data logic isn't executed if the dependency didn't trigger. In other words, with a conditions list like dep1 || dep2, then if dep1 triggers, the parameter section for dep2 is ignored and no value is set. However, it looks like ResolveParamValue() will not resolve anything if the event does not contain all dependencies.

Using 1.2.0. I don't see anything relevant in the git log since then. Version 1.3.1.

@atombender
Copy link
Author

Thanks!

@atombender
Copy link
Author

@whynowy We deployed the fix (1.4.0), and we are still experiencing this bug.

@naveenb30
Copy link

@atombender : Did you figure out any solution to this one ? I have same exact problem..Thanks in advance

@atombender
Copy link
Author

@naveenb30 No, unfortunately it still exists.

@whynowy This should be reopened.

@whynowy whynowy reopened this Dec 13, 2021
@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had
any activity in the last 60 days. It will be closed if no further activity
occurs. Thank you for your contributions.

@atombender
Copy link
Author

@whynowy As far as I know, this is still a problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants