Skip to content

Conversation

@jrafanie
Copy link
Member

@jrafanie jrafanie commented May 3, 2024

I think this simplifies the required field logic. We'll need to make sure connecting from within the runner works but this is the start of enabling that work.

image

image

image

image

@jrafanie jrafanie added the wip label May 3, 2024
if %i[classic_user classic_key].any? { |opt| attrs.key?(opt) }
attrs[:userid] = attrs.delete(:classic_user) if attrs.key?(:classic_user)
attrs[:password] = attrs.delete(:classic_key) if attrs.key?(:classic_key)
end
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 think this just goes away if classic user/key are moved to the other class as auth_key is required, right?

Copy link
Member

Choose a reason for hiding this comment

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

Not sure - @agrare ?

:isRequired => true,
:validate => [{:type => 'required'}],
},
].freeze
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 renamed these to userid and password so we don't need to define params_to_attributes here.

@jrafanie jrafanie force-pushed the split_ibm_cloud_into_cloud_and_classic_infra_credentials branch 2 times, most recently from a8fac8d to 6f86e00 Compare May 3, 2024 18:32
@jrafanie jrafanie removed the wip label May 3, 2024
@jrafanie jrafanie changed the title [WIP] Split ibm cloud credentials from ibm classic infra credentials Split ibm cloud credentials from ibm classic infra credentials May 3, 2024
@jrafanie jrafanie added the enhancement New feature or request label May 3, 2024
@jrafanie jrafanie force-pushed the split_ibm_cloud_into_cloud_and_classic_infra_credentials branch from 6f86e00 to a8356c6 Compare May 3, 2024 18:40
@agrare
Copy link
Member

agrare commented May 3, 2024

Makes sense to me, @putmanoj can you confirm this will work for opentofu-runner / ibm_cloud provider

@Fryguy
Copy link
Member

Fryguy commented May 3, 2024

Only comment I have here is I think it should it be "IBM Cloud Classic Infrastructure" vs "IBM Classic Infrastructure"? (it's this way in various strings, class names, factory names etc, so I think it would have to change everywhere)

@jrafanie
Copy link
Member Author

jrafanie commented May 3, 2024

Only comment I have here is I think it should it be "IBM Cloud Classic Infrastructure" vs "IBM Classic Infrastructure"? (it's this way in various strings, class names, factory names etc, so I think it would have to change everywhere)

My goal was to avoid confusing people by having IBM Cloud and IBM Cloud Classic Infrastructure in the select list. I saw Classic Infrastructure all over the place here and figured maybe we could use it: https://cloud.ibm.com/docs/account?topic=account-classic_keys. I'm good with whatever users would know the names to be.

@miq-bot
Copy link
Member

miq-bot commented May 8, 2024

Checked commits jrafanie/manageiq-providers-embedded_terraform@a8356c6~...2827fc2 with ruby 2.7.8, rubocop 1.56.3, haml-lint 0.51.0, and yamllint
8 files checked, 0 offenses detected
Everything looks fine. 👍

@jrafanie
Copy link
Member Author

jrafanie commented May 8, 2024

Only comment I have here is I think it should it be "IBM Cloud Classic Infrastructure" vs "IBM Classic Infrastructure"? (it's this way in various strings, class names, factory names etc, so I think it would have to change everywhere)

Ok, renamed to IBM Cloud Classic Infrastructure. The screenshots are updated.

@jrafanie
Copy link
Member Author

Ready to go

@agrare agrare merged commit 2360f12 into ManageIQ:master May 20, 2024
@jrafanie jrafanie deleted the split_ibm_cloud_into_cloud_and_classic_infra_credentials branch May 22, 2024 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants