[FR] Add skaffold inspect command for adding config dependencies

[mattsanta]

For example:
skaffold inspect config-dependencies add dependency.yaml -f skaffold.yaml

where dependency.yaml contains list of ConfigDependency that would be added to the dependencies already defined in the skaffold.yaml or added as the only dependencies if the configs in the skaffold.yaml don't have dependencies. Would expect --module support to specify which configs to adjust. This is similar to the various add commands in skaffold inspect build-env.

[ericzzzzzzz]

Just curious, why don't we use the existing skaffold inspect modules command ? By the description of this command,

"Interact with configuration modules" seems a reasonable place for your use case.

[mattsanta]

From what I can tell it seems like the subcommands under inspect are pretty specific to the use-case, which is why I added config-dependencies. To be fair, I based a lot of the idea of this as well from the inspect build-env that also edits skaffold configs and that had a module filter so thought it would make sense for this command too.

[ericzzzzzzz]

I think config-dependencies and modules refer to the same thing. To make our software cohesive, would it make more sense to include add command under skaffold inspect modules? We can still use --module to specify the target config we want to change if we add add under skaffold inspect modules.

It feels surprise that we have config-dependencies and modules for skaffold inspect at the same time. For example, if we want get a list of config-dependencies in a module, do we use skaffold inspect config-dependencies list or skaffold inspect modules list? Although, this feature is for a specific use case, but we're not just building software for ourselves right?

[mattsanta]

Could you give me your thoughts on what you think this would look like in the case we go with skaffold inspect modules add?

Some questions I have:

• Is the input still a file that defines the config dependencies to inject? Example file:

- configs: ["cfg1]
   git:
     repo: some-repo
     path: some-path
- googleCloudStorage:
     source: some-source
     path: some-path

• Does it make sense still even though the config dependencies may be injected into multiple modules? In the case that a module isn't specified it'd apply to all of the ones in the skaffold file.
• In your opinion, if we could go back would skaffold inspect build-env add commands actually be under some form of skaffold inspect modules add? I ask this because I feel like this is very similar.

Definitely want to do this right and you're more familiar with the product so your opinions matter to me. Thanks!

[mattsanta]

@renzodavid9 can you provide your thoughts as well since you reviewed the PR I put up?

[ericzzzzzzz]

• Is the input still a file that defines the config dependencies to inject

  • My preference on using a file vs a few flags as input is almost neutral
  • Using a file has two advantages
    • reusability/portability the same input file can be used multiple times in many places
    • you get what you see with the assumption that the content from the input file use the same schema as skaffold module
  • Using flags to provide data
    • It provides skaffold more flexibility to manipulate data and no skaffold version/schema version conflicts
  • I think how we expect people to use this feature would help us make the decision
    • Do we expect humans to call this directly to change their configs? Then input as a file loses the reusability as its advantage this may be just a one-off thing for devs. For the other point, 'you get what you see', why users don't just edit their skaffold config directly then? I don't see much benefits for using file as input in this case.
    • Do we expect a program to use this to inject deps at runtime? It is probably easier to have a similar model defined in the program then persist it as a yaml/json file, but if this is used by a program, it isn't too bad to feed data with flags either.

• I feel it more intuitive to just inject modules into default configuration, if target module is not specified. It doesn't make much more sense to inject modules into multiple configurations. If we do so, what is the expected behavior when running skaffold render, skaffold apply flow and what is the expected behavior for skaffold run flow? For skaffold-render, skaffold-apply injecting a module to multiple configuration seems almost no differences than just injecting into default configuration when running apply, for render it may the change render order. For skaffold run, it could change deploy order for injecting into default configuration vs injecting into multiple configurations. It doesn't really help anything to inject modules into multiple configuration about creating confusion.

• Sorry, I'm not sure I understand this, but the point of my original comment is that if we want to operate on a resource, we should group the operations on that resource under the same command.

[mattsanta]

Discussed over a meeting with @ericzzzzzzz and @renzodavid9.

Conclusion:
Will create the functionality under skaffold inspect modules add and will continue to provide the dependencies to inject via a file. The file will be a json object as opposed to the skaffold yaml for config dependencies to avoid having to deal with changes related to the yaml structure in the future.

Some additional notes, this command is expected to run before skaffold diagnose and will inject the dependencies in all the Configs in the skaffold file unless a specific module is specified.

[ericzzzzzzz]

@mattsanta Sorry, after doing a little bit more investigation, I think we might want to go back to use config-dependencies , module actually refers to a specific configuration, it was supposed to be named as config in flag, but when we implemented multi-configs feature, that --config has already been used for providing skaffold global configuration details see here . I thought module should be a project that can has multiple pipeline/multiple configuration, I was wrong. I'm so sorry for the noise.

[mattsanta]

Updated my PR. @renzodavid9 FYI ^.