Create Asset Generation CLI Enhancement #214
Conversation
Signed-off-by: JonahSussman <[email protected]>
JonahSussman
force-pushed
the
JonahSussman-patch-2
branch
3 times, most recently
from
9a9ce03 to
a8e6c48
Compare
Signed-off-by: JonahSussman <[email protected]>
JonahSussman
force-pushed
the
JonahSussman-patch-2
branch
from
a8e6c48 to
34096f5
Compare
| We propose modifying the existing Kantra CLI instead of developing an entirely | ||
| new CLI to ensure a smoother adoption process for users. Leveraging Kantra's |
There was a problem hiding this comment.
For phase 1, integration into existing MTA/Kantra CLI was set as non-goal. The idea, if I'm not wrong, is to be independent to other projects to deliver faster the 1st phase.
Do you think that having a simple CLI for the phase 1 will overcomplicate a future integration?
There was a problem hiding this comment.
The approach I'm envisioning is to integrate with the existing Kantra from the beginning to ensure we can deliver value to customers more quickly, as they already will have Kantra vetting in their security processes.
The core functionality will be implemented as a standalone Go library, which will serve as the foundation for this enhancement. This ensures that the Kantra integration remains a thin frontend layer while keeping the library modular and reusable for future projects.
| - Create a standalone library for integration with other tools and workflows. | ||
| - Extend Kantra to support: | ||
| - `discover` command: Output a YAML/JSON representation of Cloud Foundry resources. | ||
| - `generate` command: Produce Helm charts for deployment to Kubernetes. |
There was a problem hiding this comment.
I think generate can produce values.yaml (generated from the canonical form) and an example helm template. That helm template will need to be tuned by the user.
It's too complex to generate decent helm charts.
| - `generate`: This command takes a canonical representation and a Helm | ||
| template to produce deployment-ready Helm charts. Details: | ||
| - **Flags and Options:** | ||
| - `--input=<file>` (required): Specifies the canonical configuration file |
There was a problem hiding this comment.
I'd like to have the option to provide a CF manifest as input. IMO, This will be useful if a user provides the CF manifest directly, for testing purposes and during the development.
There was a problem hiding this comment.
I think this overlaps with the purpose of the canonical configuration. The idea is that the users wouldn't need to rely on the original platforms input formats once it is generated.
Is there a specific use case you're thinking that the canonical config doesn't cover for generate?
| - Implement the core functionality as a standalone library in Go for reuse in | ||
| Kantra and other projects. |
There was a problem hiding this comment.
Can you clarify this? It seems different from the Motivation section
There was a problem hiding this comment.
The idea is that the Kantra CLI will use the library, but other projects can use the library as well if they want to
Signed-off-by: Savitha Raghunathan <[email protected]>
| disk), and deployment settings. | ||
| - **Error Handling:** | ||
| - Detect and report missing or malformed input files with detailed error messages. | ||
| - Provide retry options for transient errors during discovery. |
There was a problem hiding this comment.
This could be addressed by the user by evaluating the exit code and retrying when it's not 0. I think adding a retry logic is a complexity we might want to avoid as it adds burden to our cli when it does not guarantee it solves the problem.
| application name, instances, routes), runtime information (e.g., memory, | ||
| disk), and deployment settings. | ||
| - **Error Handling:** | ||
| - Detect and report missing or malformed input files with detailed error messages. |
There was a problem hiding this comment.
What do you have in mind? a structured error output? or just free text as error? Should the CLI exit when an error occurs or do you think it should consolidate all the errors and output them together as well as any generated file?
| - Validates the generated artifacts for completeness and correctness. | ||
| - **Error Handling:** | ||
| - Log errors when applying Helm templates. | ||
| - Support a dry-run mode to identify potential issues without generating artifacts. |
There was a problem hiding this comment.
Do we need to allow server side checks when generating non-yaml/json templates, such as Dockerfile or EAP xml configuration files?
There was a problem hiding this comment.
I don't think I understand. Could you phrase it differently?
There was a problem hiding this comment.
@jordigilh that would mean writing some custom validators. XML should be quite straightforward as we have XSD schemas, but other files such a Dockerfile would be a bit trickier. Sounds like a nice to have for further iterations, but I'd move forward without it for the moment.
| Kubernetes. | ||
| - Validates the generated artifacts for completeness and correctness. | ||
| - **Error Handling:** | ||
| - Log errors when applying Helm templates. |
There was a problem hiding this comment.
What do you mean by applying Helm templates? Do you think the CLI should render the helm templates? or just render the non-yaml/json templates?
There was a problem hiding this comment.
I should be more specific. The errors encountered in the generation step, whether that be from helm generators or others, should be logged via the CLI. WDYT?
Signed-off-by: Savitha Raghunathan <[email protected]>
adding discovery agents
|
|
||
| - Create a standalone library for integration with other tools and workflows. | ||
| - Extend Kantra to support: | ||
| - `discover` command: Output a YAML/JSON representation of Cloud Foundry resources. |
There was a problem hiding this comment.
Should discovery be used for gathering platform info rather than output? It mentions here output of CF resources, but maybe that could be another step after discovery? (e.g. discovery -> runOutPut(?) -> generate). wdyt?
There was a problem hiding this comment.
is there a reason we would need an intermediate step? Right now the discovery command is defaulting to standard out and provides an option to write to a file.
| Kubernetes. | ||
| - Validates the generated artifacts for completeness and correctness. | ||
| - **Error Handling:** | ||
| - Log errors when applying Helm templates. |
There was a problem hiding this comment.
It may be nice send helm-specific logs/errors to a log, and keep the cli errors separate.
There was a problem hiding this comment.
If we provide structured logs, that should be an easy task to filter.
|
|
||
| - Handling of sensitive resource data, such as secrets, is explicitly out of | ||
| scope for this CLI. Instead, we will rely on Konveyor's secure encryption | ||
| stores, which users already use with Konveyor. |
There was a problem hiding this comment.
Encryption should be outside the scope of the CLI. However, the CF provider in the CLI is the only thing that understands CF and can identify which values are sensitive. Thinking the canonical manifest (schema) should have a secrets section. When the CLI identifies a sensitive value, it would add an entry to the secrets section containing the sensitive data. The sensitive value would be replace with a reference to the secret.
For example (something like):
CF manifest
password: ABC
canonical manifest:
application:
password: $(password)
secrets:
- name: password
- value: ABC
When the manifest is stored in konveyor, the secret values could be encrypted.
Naming is for example.
There was a problem hiding this comment.
@jordigilh @gciavarrini is this something we can add to #215 ? Have we identified anything sensitive in the CF spec that should be stored encrypted in the hub in subsequent iterations?
There was a problem hiding this comment.
CF app manifest exposes the docker username as a field in the spec. I would consider that to be sensitive data. Right now we're capturing it as a field in the canonical form, but we can follow on what @jortel suggest and store it in a separate section, although it would be stored in a clear format. Let me know if that works for you and I'll update both the logic in the discovery and the enhancement.
Throughout our research on CF, we've seen across a few examples where credentials (username + password) were injected as environment variables in the parameter field of the service in free text format (k/v) for registering services. The value can be structured (json) but it's dependant on the target service and we would not be able to parse and extract the information in all cases. I would suggest to let the users clean up the values after the canonical form is generated to suit their deployment needs for their cases, since the service registration in CF does not follow the same pattern as in K8S services.
|
Can we add an example: --help output so we can see the proposed/consolidated subcommands and options. |
| charts). The enhancement will provide a foundational capability for platform | ||
| migrations, enabling a tech-preview release in an upcoming version of Kantra and | ||
| integration into downstream workflows. | ||
|
|
There was a problem hiding this comment.
@JonahSussman ,
How would that scale in case we have many applications with same/overlapping run time requirements?
Also. would we check on Kubernetes side whether the required resources are there, and in this case no asset generation will be required?
There was a problem hiding this comment.
@istein1 We will be mapping the canonical values to the helm template provided by the user. Any validation and verification of generated assets/manifests are out of scope for this MVP.
| overall workflow for existing users already familiar with the tool. To achieve | ||
| modularity and maintainability, we will create a standalone library of | ||
| functionality to which the CLI will serve as a frontend, allowing for future | ||
| integration with other tools and workflows. |
There was a problem hiding this comment.
Hi @JonahSussman , @savitharaghunathan , What CLI would the new standalone library for asset generation and platform awareness work with ? Containerized CLI or Containerless CLI ?
There was a problem hiding this comment.
This does not use analysis; so it's not a matter of containerless vs container CLI in this context. These are new commands.
There was a problem hiding this comment.
I think this is a valid question though, do we expect to require a container runtime for these new capabilities to execute? I would say we need to avoid that as much as possible. It is important that these new capabilities can be executed in a Tekton Pipeline.
There was a problem hiding this comment.
No, no containers should be executed. Sorry for the confusion
| overall workflow for existing users already familiar with the tool. To achieve | ||
| modularity and maintainability, we will create a standalone library of | ||
| functionality to which the CLI will serve as a frontend, allowing for future | ||
| integration with other tools and workflows. |
There was a problem hiding this comment.
This does not use analysis; so it's not a matter of containerless vs container CLI in this context. These are new commands.
| - Create a standalone library for integration with other tools and workflows. | ||
| - Extend Kantra to support: | ||
| - `discover` command: Output a YAML/JSON representation of Cloud Foundry resources. | ||
| - `generate` command: Produce Helm charts for deployment to Kubernetes. For the mvp, |
There was a problem hiding this comment.
Based on my new understanding of how this works; I wonder if we should instead call this render? Under the hood we are essentially passing in a location to a helm chart and rendering the templates. I don't know; I originally liked generate
There was a problem hiding this comment.
did we decide on the naming? @dymurray
There was a problem hiding this comment.
discovery manifest I think is what we agreed upon
There was a problem hiding this comment.
@dymurray ack. For the command, are we good with generate ?
| - `generate` command: Produce Helm charts for deployment to Kubernetes. For the mvp, | ||
| the we will generate a values.yaml file based on the CF discovery manifest, | ||
| and combine it with the user's templates to generate a helm chart | ||
| - Utilize the [canonical configuration enhancement](link: TODO) to serve as a consistent |
There was a problem hiding this comment.
The more I understand about this; there is no "canonical config". discover spits out a values.yaml and the user puts that into their helm chart; then calls generate with a path to a helm chart where the templates get rendered.
There was a problem hiding this comment.
There shouldn't be a need for the user to put the values file in the chart. That should be an input parameter for the CLI, and it will make sure it is passed correctly to the templating engine. Users shouldn't have to move files around for this to work.
|
|
||
| - `generate`: This command takes a canonical representation and a Helm | ||
| template to produce deployment-ready Helm charts. Details: | ||
| - **Flags and Options:** |
There was a problem hiding this comment.
I would add some additional options here. I just want to convey the functionality, I'm open to changes in names and format:
--set key=value: Sets a value for the given in the command line. If the key already exists in the values file that was passed, it will override its value.--non-k8s-only: Only renders the non-k8s templates available in the files/konveyor path from the target Helm Chart.
There was a problem hiding this comment.
--set key=value: Sets a value for the given in the command line. If the key already exists in the values file that was passed, it will override its value.
@rromannissen - this will override the value for the key in discovery manifest, which in turn will override the values.yaml file in the given helm template. is that correct?
There was a problem hiding this comment.
Not really. The discovery manifest and the values.yaml file would remain the same, but when rendering the value of the given key would be overridden by the given value. This is similar to the --set flag from the helm template command.
There was a problem hiding this comment.
I updated the flags. Thanks @rromannissen for the clarification
| - Validates the generated artifacts for completeness and correctness. | ||
| - **Error Handling:** | ||
| - Log errors when applying Helm templates. | ||
| - Support a dry-run mode to identify potential issues without generating artifacts. |
There was a problem hiding this comment.
@jordigilh that would mean writing some custom validators. XML should be quite straightforward as we have XSD schemas, but other files such a Dockerfile would be a bit trickier. Sounds like a nice to have for further iterations, but I'd move forward without it for the moment.
dymurray
changed the title
| supports helm. Defaults to `Helm` | ||
| - `--set key=value` (optional): Sets a value for the given in the command line. | ||
| If the key already exists in the values file that was passed, it will override its value. | ||
| - `--non-k8s-only` (optional): Only renders the non-k8s templates available in the files/konveyor |
There was a problem hiding this comment.
How does this flag work when the type generator is not helm? We know that for helm we will have to render the files from a directory inside the helm chart, but is that also applicable for other template engines? To me this flag is helm specific and it's being exposed at the same level as the template engine flag.
Maybe the type value should be a command instead of a flag?
kantra generate helm --template ./ --output-dir /tmp --non-k8s-only
and then if we were to use ansible:
kantra generate ansible --playbook ./ --output-dir /tmp
It looks cleaner since the flags specific for helm (template) are not applicable to ansible (playbook for instance).
WDYT?
| specified platform and generating a canonical representation in YAML | ||
| format. Details: | ||
| - **Flags and Options:** | ||
| - `--platform=<platform>` (required): Specifies the platform to discover. |
There was a problem hiding this comment.
Same thing here: PCF applications might require to pass specific arguments to enable discovery (e.g. performing discovery over live platform). I suggest to turn this flag as a subcommand to allow specifying flags per platform:
$> kantra discover cloud-foundry --use-live-connection --username <username> --password <password> --server-url https://localhost:8443Common flags can remain as part of the discovery command, but specific ones will fall into each sub-command/platform.
No description provided.