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

Templating everywhere discussion #9063

Open
ericzzzzzzz opened this issue Aug 31, 2023 · 10 comments
Open

Templating everywhere discussion #9063

ericzzzzzzz opened this issue Aug 31, 2023 · 10 comments

Comments

@ericzzzzzzz
Copy link
Contributor

Some background information:
#9062 (comment)

Hi @briantopping , let's move our discussion about using templates for all fields to here, so we can be more focused on this issue.

Cloud you clarify this?
is there some place that the original static (configuration) context as well as current dynamic context can be accessed in a read-only manner?

as to adding the default key in the dot context for every script, changing depending on it's usage

  • Can this be achieved by skaffold profile feature? Having a default(main) profile, and replace necessary values with other profiles. Profiles can also be activated automatically based on environment variables.
@briantopping
Copy link
Contributor

Thanks Eric!

Contexts

From the template doc:

The name of a key of the data, which must be a map, preceded by a period, such as .Key The result is the map element value indexed by the key. Key invocations may be chained and combined with fields to any depth: .Field1.Key1.Field2.Key2 Although the key must be an alphanumeric identifier, unlike with field names they do not need to start with an upper case letter. Keys can also be evaluated on variables, including chaining: $x.key1.key2

My thought is the full context of the current Skaffold invocation should be maximally accessible in a read-only way. My previous writing conflates default and maybe should be .default. The former is easier to write but means it's a function and not part of the context provided to template.Execute() in the data parameter.

Those semantics aside, having the full invocation context available to templates would mean Skaffold users could reliably address any runtime value. For instance, if ko generates an artifact hash, a template in deploy should be able to access it with something like a yq query. I can copy the path of a value in a complex YAML in Jetbrains products with a right-click context menu on the item I need to reference. That could be pasted into this template lookup very quickly and intuitively.

Nested Template Definitions

It's also worth considering named templates here. template.ExecuteTemplate() has the provision to name a template. If we do end up precompiling and caching templates, they could be cached by the yq address in the skaffold.yaml. This would enable nested template definitions to work.

@ericzzzzzzz
Copy link
Contributor Author

Hi, @briantopping Thank you for the explanation.The idea of a template being able to access the values generated in runtime with yq is very interesting , but things will become much more complicated in the context multi-modules projects. I feel that there will be many challenging and interesting problems related to this feature are worth discussing. I believe it is necessary to have a design doc for it to guide the implementations. @renzodavid9 talked about this in our previous team meeting, and I‘m also happy to continue this conversation. However, given that our team is currently understaffed, this feature request may not be prioritized in the near future. I'd appreciate someone from the community can help with the design/implementation.

@briantopping
Copy link
Contributor

Hi Eric, as I mentioned, I'd like to work on this. Is there some process I can start following to reduce overhead to existing team members?

@ericzzzzzzz
Copy link
Contributor Author

ericzzzzzzz commented Sep 1, 2023

That would be great! Thank you so much! Here is our pull request guidelines https://github.com/GoogleContainerTools/skaffold/blob/v1/docs/community/small-prs.md
and design proposal template
https://github.com/GoogleContainerTools/skaffold/blob/v1/docs/design_proposals/design-proposal-template.md

@girstenbrei
Copy link

@briantopping Im very interested in this, too. I took a look at solving #9062 and one thing I noticed was that the templates are instantiated every time they are executed. Basically every templated field calls template.New (and re-parses environment variables).

This also leads to different variables being available in different fields, different info is provides on template execution.

Currently build information seems to be computed from builds []graph.Artifact on demand in ConstructOverrideArgs.

Actually this now sounds very tangled with #9062 except the "what is provided by previous stages and how", but very connected by "how to do anything with it then".

I'd rather do it right one time before messing with templating from different locations to just fix the default function issue. I can push you my experiments on monday. Open to support you!

@girstenbrei
Copy link

So I setup #9065, happy to receive any feedback on it (or to just use it as a basis to steal ideas from)

@briantopping
Copy link
Contributor

briantopping commented Sep 4, 2023

Hi @girstenbrei, thanks for the note and the work in #9065. We are having our equivalent of May 1 here, so I won't be able to get too deep here. I'm thrilled to have more attention to this issue as it opens up a lot of user-based flexibility with their Skaffold configurations.

I wonder if we could create a plan first. You may have more time than me to get working on this, so I won't be coming in and competing to implement ideas that we plan out. At the same time, I think you are seeing that this is a pretty comprehensive change. It will get increasingly difficult if we need to rewrite it several times to get to the final objective, especially if we don't know what that objective is or it's a moving target that we are implementing separately.

To use an example, as I discussed above, parsing the templates and caching them keyed by yq address will create additional capabilities beyond allowing "templates everywhere". Templates create code, code needs to be kept DRY. I have a Skaffold environment containing ten copies of the same template in it. Being able to define it once and use it in all ten places is far more maintainable.

We also have to be conservative with the time of the Skaffold team. @ericzzzzzzz referenced this template and regardless of who is our sponsor on this journey, they are going to want to know what we are proposing in plain language before they are asked to review code. Getting signoff for a proposal early means we can invest in the complete implementations, including test cases. In that case our sponsor can move ahead with simply committing the changes and we can move on to additional steps in sequence to get to the overall goal.

If we don't start with a plan, I'm afraid that we will lose the commitment of the Skaffold team. I have very limited commits on this project and experience with the team, but I do know they take the project very seriously and have limited time to do so.

Interested in your thoughts here. I know this is more involved than just writing code, but Skaffold has a lot of moving parts and we have to be very careful with something that touches every config field like this effort proposes.

@girstenbrei
Copy link

Thank you very much for the input @briantopping . I created #9070 containing a first shot at the proposal trying to lay down a plan and listing open questions.

I deliberately split out the topics how templating is used in code, which fields are available to template and which values one can template with. Although all are related to this issue and to each other, IMHO starting with how templating is used can be a suitable start. But I'm interested in your thoughts here, maybe we should do all three topics in the one design proposal, but implement them one by one?

Happy to hear about any implementation hints, too. I'm not that good with golang nor do I know the code basis well.

@briantopping
Copy link
Contributor

Sure, that's great to hear. My personal preference is that we plan for the target state of how we'd like to see the template experience in something like an 18 month window, then work backwards with steps that can get us there.

We should intend to implement everything ourselves, but by doing so as smaller standalone PRs that provide individually valuable functionality, we reduce the effort and risk for the Skaffold team. We want them to be able to accept our commits with reasonable effort, but not feel like the code is compromised until the last PR is accepted.

Please don't worry about doing this alone, I only wanted you to have confidence that this is a team effort. Hopefully we'll each be able to exceed each other's expectations and this will be fun and rewarding for everyone.

It's 0320 here, will pick this up again when I have some more rest. Cheers!

@girstenbrei
Copy link

girstenbrei commented Sep 8, 2023

Thanks for the encouragement! I opened up the template to cover a more complete view on how templating IMHO could be. Feel free to let me know about any questions / suggestions / issues you have with the proposal so we can get a solid plan.

And I hope I don't keep you from sleeping again, I'm not in that kind of hurry 😁 , Greets!

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

No branches or pull requests

3 participants