Skip to content

Fix input_vars fetching from job options in Job #104

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

Merged
merged 3 commits into from
Jun 10, 2025

Conversation

putmanoj
Copy link
Contributor

Fix the fetching of :input_vars from job options,

Fixes #103

@putmanoj putmanoj changed the title Fix input vars in job Fix input_vars fetching from job options in Job Jun 10, 2025
@putmanoj putmanoj force-pushed the fix-input_vars-in-job branch from 196a1b2 to 7aea7db Compare June 10, 2025 12:04
@putmanoj putmanoj marked this pull request as ready for review June 10, 2025 12:11
@putmanoj putmanoj force-pushed the fix-input_vars-in-job branch from 3870a9a to 2167c7c Compare June 10, 2025 13:21
@agrare agrare self-assigned this Jun 10, 2025
@agrare agrare added bug Something isn't working spassky/yes? labels Jun 10, 2025
Comment on lines 93 to 100
:credentials => [],
:credentials => anything, # Authentication.where(credentials),
Copy link
Member

Choose a reason for hiding this comment

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

I think we should try to match the expected credentials here not anything and comment this out

Copy link
Member

Choose a reason for hiding this comment

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

Interesting this actually passes with the old [] still was there a reason you changed this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

reverting back to []

It started failing for spy function - https://github.com/putmanoj/manageiq-providers-embedded_terraform/blob/fix-input_vars-in-job/spec/models/manageiq/providers/embedded_terraform/automation_manager/job_spec.rb#L106
in between working on the issue ..., in logs could see credential as ActiveRecord relation, ....
also being credential is empty list .. putting anything seems to fix it the problem then,

@@ -178,7 +178,7 @@ def input_vars_type_constraints
end

def input_vars
options.fetch(:input_vars, {})
Copy link
Member

Choose a reason for hiding this comment

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

Why was this not working previously? Was the issue that the input_vars passed in above had another nested input_vars ? Maybe if you have an example of the previous payload

Copy link
Member

Choose a reason for hiding this comment

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

Okay I see the payload in the issue

:input_vars=>{
    "execution_ttl"=>"", 
    "verbosity"=>"0", 
    "input_vars"=>{
        "name"=>"World"
    }
},

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, after changes for 'extra_vars' removal , .., as these test were using simulated values ..., tests gave a false positive result

@miq-bot
Copy link
Member

miq-bot commented Jun 10, 2025

Checked commits putmanoj/manageiq-providers-embedded_terraform@72f7f7b~...18ea6f8 with ruby 3.1.5, rubocop 1.56.3, haml-lint 0.62.0, and yamllint
3 files checked, 0 offenses detected
Everything looks fine. 🍪

@agrare agrare merged commit 08c48c2 into ManageIQ:master Jun 10, 2025
3 of 4 checks passed
@Fryguy
Copy link
Member

Fryguy commented Jun 10, 2025

Backported to spassky in commit 7e54d61.

commit 7e54d61caf94dc5b51e052cffc084806d4b70ed9
Author: Adam Grare <[email protected]>
Date:   Tue Jun 10 10:36:11 2025 -0400

    Merge pull request #104 from putmanoj/fix-input_vars-in-job
    
    Fix input_vars fetching from job options in Job
    
    (cherry picked from commit 08c48c272cbdfe2cd7b7c6eabf22068c6ec4971e)

Fryguy pushed a commit that referenced this pull request Jun 10, 2025
Fix input_vars fetching from job options in Job

(cherry picked from commit 08c48c2)
@putmanoj putmanoj deleted the fix-input_vars-in-job branch June 10, 2025 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working spassky/backported
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The input parameter for Terrraform:Runner.run is not correctly passed from EmbeddedTerraform Job.
4 participants