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

Make the value in the resource configuration source an apiextension JSON object instead of string #320

Open
Skarlso opened this issue Nov 8, 2024 · 4 comments
Labels
area/ipcei component/ocm-controllers OCM Controllers kind/enhancement Enhancement, improvement, extension

Comments

@Skarlso
Copy link
Contributor

Skarlso commented Nov 8, 2024

What would you like to be added:

We have this:

type ConfigurationRuleYAMLSubsitutionSource struct {
	// Value is the value that will be used to replace the target in the file.
	Value string `json:"value"`
}

Should be something like this:

type ConfigurationRuleYAMLSubsitutionSource struct {
	// Value is the value that will be used to replace the target in the file.
	Value *apiextensions.JSON `json:"value"`
}

To allow for arbitrary json values.

Why is this needed:

@Skarlso Skarlso added the kind/enhancement Enhancement, improvement, extension label Nov 8, 2024
@Skarlso Skarlso moved this from 🆕 ToDo to 📋 Next-UP in OCM Backlog Board Nov 8, 2024
@Skarlso Skarlso added the component/ocm-controllers OCM Controllers label Nov 8, 2024
@jakobmoellerdev
Copy link
Contributor

I believe the yaml substitution engine is not powerful enough yet to handle structure replacement of fields. We will have to think about how we go with this. Also, since the localization is not technically part of the OCM lib, we should probably move the entire logic out into the controllers as another task. WDYT?

@Skarlso
Copy link
Contributor Author

Skarlso commented Nov 14, 2024

I'm fine with moving it, but this is isn't working right now, and I don't have another option of just plain exchanging a number.

The value being json probably isn't working, but then we'll have to rethink how we do the substitution. Because right now, only strings are working. :)

@jakobmoellerdev
Copy link
Contributor

lets split this into 2. parts:

  1. allowing numbers (that one we can support simply with the change you suggested)
  2. allowing arbitrarily nested structs (that requires the lib change

WDYT?

@Skarlso
Copy link
Contributor Author

Skarlso commented Nov 14, 2024

Sounds good to me 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ipcei component/ocm-controllers OCM Controllers kind/enhancement Enhancement, improvement, extension
Projects
Status: 📋 Next-UP
Development

No branches or pull requests

2 participants