custom core packages #180

nabuskey · 2024-03-19T18:28:28Z

Currently, it's impossible to customize core packages on startup.

Let's take an example. For a user to configure a very common case of customizing ArgoCD config map to allow for custom health checks:

---
apiVersion: v1
kind: ConfigMap
metadata:
  name: argocd-cm
  namespace: argocd
data:
  resource.customizations: |
    argoproj.io/Application:
      health.lua: |
        hs = {}
        hs.status = "Progressing"
        hs.message = ""
        if obj.status ~= nil then
          if obj.status.health ~= nil then
            hs.status = obj.status.health.status
            if obj.status.health.message ~= nil then
              hs.message = obj.status.health.message
            end
          end
        end
        return hs

To apply this in idpbuilder, users must:

Start idpbuilder and wait for it finish.
Go to gitea GUI or clone the argoCD repository.
Edit the CM file and push.

Obviously this workflow is not portable. I think we need to introduce a way to customize core packages.

What I am proposing in this PR is introduction of the package-custom-file flag (I don't like this name, please suggest something better). This flag looks something like this:

$ idpbuilder create --package-custom-file=argocd:pkg/k8s/test-resources/input/argocd-cm.yaml

$ cat pkg/k8s/test-resources/input/argocd-cm.yaml

apiVersion: v1
data:
  application.resourceTrackingMethod: annotation
  resource.exclusions: |
    - kinds:
        - ProviderConfigUsage
      apiGroups:
        - "*"
kind: ConfigMap
metadata:
  labels:
    Test: Data
  name: argocd-cm

$ kubectl get cm argocd-cm -n argocd

apiVersion: v1
data:
  application.resourceTrackingMethod: annotation
  resource.exclusions: |
    - kinds:
        - ProviderConfigUsage
      apiGroups:
        - "*"
kind: ConfigMap
metadata:
  annotations:
    argocd.argoproj.io/tracking-id: argocd:/ConfigMap:argocd/argocd-cm
  creationTimestamp: "2024-03-21T18:03:14Z"
  labels:
    Test: Data
  name: argocd-cm
  namespace: argocd

With this flag, you don't have to specify the entire manifest for a package. We will override provided objects (GVK + NS + Name match). E.g. if you just want to change the argocd-cm CM, you can just provide that.

The provided file (/tmp/cm.yaml in above example) can be a go template file. Core customization flag values (hostname, protocl, etc) are used to render it before applied to the cluster.

In addition, you can specify additional objects like more CMs and secrets for ArgoCD to consume.

I am not sure if we want to support resource deletion initially but I imagine we will need to support it.

jessesanford · 2024-03-20T08:34:27Z

I like the idea, it might be helpful to have a command to print the built in manifests so that folks can understand the defaults and diff between theirs and what's built in. Otherwise they have to wait for a whole create run to come up just to see what the values are or to read the sources. Either is not that bad, but if we are trying to be friendly :)

nabuskey · 2024-03-21T18:14:48Z

it might be helpful to have a command to print the built in manifests so that folks can understand the defaults and diff between theirs and what's built in.

agreed. I think we need the export command like we discussed before.

nimakaviani · 2024-03-26T05:30:50Z

this is interesting, thanks for the proposal.

skimming over the current implementation, looks like we will do a full override for a given resource from the embedded package file, if the flag is present. I wonder if we can use an overlay strategy (i.e. Kustomize) to allow folks to add strategic patches instead, so we could change bits in a target file more surgically but also based on some common and already existing patterns. since we have a known list of files for each of the core components, constructing an internal base should be feasible, I think.

nimakaviani · 2024-04-02T15:46:05Z

my suggestion also goes hand in hand with this proposal #165

nabuskey · 2024-04-11T18:15:58Z

I don't think kustomize is a good fit.

It requires end users' kustomize knowledge to make changes to core packages.
Strategic merge cannot handle all manifest manipulation cases. For example, to remove a field or manipulate arrays, you need other transformers. This means users need to write a full kustomize spec

So if we use kustomize, we are asking the users to:

Get the correct version of manifests from our repo.
Make changes to kustomize so that generated manifests are exactly what they want by running kustomize build.
Maintain kustomize transforms. For example, if they want to remove an array item at a specific position, they need to ensure they have not been changed in the base version before they can start using another version of idpbuilder.

I think, in most cases, user made changes to core packages are not that complex. e.g. additional argocd or ingress nignx CMs. While I agree it's a good idea to use an industry standard tools, I am not sure if they are worth it (in terms of implementing such logic and user effort) just to make small changes like these.

cmoulliard · 2024-04-12T13:43:57Z

Obviously this workflow is not portable. I think we need to introduce a way to customize core packages.

I fully agree. This is today a big issue that we have around the customization of the packages deployed on a cluster as some parameters can be easily configured when we deploy the existing resources (helm, kustomize, etc) but some will require that a user knows how the package A can be customized, etc and the argocd example you provided is a good one ;-)

Another example could be: How can I create some users/pwd for gitea, repositories, etc

cmoulliard · 2024-04-12T13:48:01Z

With this flag, you don't have to specify the entire manifest for a package. We will override provided objects (GVK + NS + Name match). E.g. if you just want to change the argocd-cm CM, you can just provide that.

What will happen if the resource already exist ? Will idpbuilder by default merge or replace a CM, secret, etc ? This is an important point that we should carefully log (and why not include as annotations) to inform the users about what it happened

cmoulliard · 2024-04-12T13:53:58Z

What you propose here is inline with my ideas where we should in a future release propose a way for an admin users to define using "profiles/stacks" what they would like to deploy on cluster A or B or C where a profile / stack will be composed of x packages: package A version 1.2.3, package B version 3.4.5, etc and where we will propose a way to customize each package

Remark: Personally and to avoid to execute too many manual commands, I was thinking to use Tekton, Argocd flow or any other flow engine (https://sonataflow.org/serverlessworkflow/latest/index.html) to manage using tasks post installation steps or rollback

nabuskey · 2024-04-12T15:38:18Z

Another example could be: How can I create some users/pwd for gitea, repositories, etc

The admin credentials for gitea can be configured using current implementation in this PR. You could probably create repositories as well, though I have not tried. From security perspective, we should generate gitea admin credentials instead of using hardcoded values.

What will happen if the resource already exist ? Will idpbuilder by default merge or replace a CM, secret, etc ? This is an important point that we should carefully log (and why not include as annotations) to inform the users about what it happened

It shouldn't touch already existing k8s resources belonging to core packages. As of now, we never touch the manifests for core packages once they are installed. This hasn't changed. We should 100% log (debug level) what was replaced though.

cmoulliard · 2024-04-15T09:15:37Z

It shouldn't touch already existing k8s resources belonging to core packages.

I like the idea about to extend the existing package definition (= what Argocd deploys using the inputs coming from helm chart (or kustomize or etc) instead of merging as that will improve the readability about what idpbuilder do.

Another benefit will also be (according tor the scenario which finally correspond to the users's needs declared in profile/stack), that different configs could be applied against different deployments (cluster a or be or c) using perhaps the same idpbuilder release: 1.2.3 or 1.2.5 or etc

nimakaviani · 2024-04-16T07:06:36Z

It shouldn't touch already existing k8s resources

ah ok. I was under the impression that people can modify the already existing secrets or CMs as well. if it is only for the first run, then having a flag to pass in the full config makes sense. But why only at create time? in terms of its usability also, only allowing the flag to work at create time will offer a disjoint experience IMO. Basically, your first try with --create gets the new config applied, but your followup tries with --create wont have an impact, correct?

then what will the cli do in followup runs with both the --create and the --package-custom-file flag present? fail? if so, do we expect users to do a --recreate in case of any change to the config map? also do they have to remove --package-custom-file after the first run if they want to do some reconciliation as they do the development work?

cmoulliard · 2024-04-16T07:33:59Z

ah ok. I was under the impression that people can modify the already existing secrets or CMs as well.

We should be very careful here about what we propose:

If the goal is to change what we install using argocd + helm/kustomize etc then users will certainly complain and tell us: "Why don't you use the secret/cm/etc part of the github repo that we created/customized to create a new IDP builder". Such appraoch is risky and will confuse folks like also increase the debugging process or maintainance.
If the goal is to apply some "configs" post idpbuilder created, then such option makes sense as it will not hurt what idpbuilder installs or what the user defined as platform to be provisioned. In this case, such additional configs should be part of a "profile/stack" definition (remember my screenshots) and applied against a specific cluster, etc

nabuskey · 2024-04-16T16:40:24Z

During the community meeting we discussed what we want to do with this.

With the current implementation, modifications to CORE packages are applied at creation only. This was done to ensure we do not break the current behaviour. E.g. We currently do not touch CORE package manifests stored in Gitea repositories after installation. This way users can make modifications in the Gitea UI or clone locally then push without worrying about manifests being overwritten.

Most people in the meeting seem to think we should modify them if specified in the CLI. As a consequence, any modifications made in Gitea UI will be lost.

I don't have a preference either way. So I'm going to let it modify manifests post creation.

nabuskey · 2024-04-17T21:46:55Z

This is now ready for review.

IF you want to test it, flow is

idpbuilder create
Wait for it to finish.
Go to https://gitea.cnoe.localtest.me:8443/giteaAdmin/idpbuilder-localdev-argocd. verify the argocd-cm doesn't have any additional labels.
Run idpbuilder create --package-custom-file=argocd:pkg/k8s/test-resources/input/argocd-cm.yaml.
Verify argocd-cm has a test label.

Signed-off-by: Manabu McCloskey <[email protected]>

nimakaviani

LGTM.

One thought I had as was reviewing the PR was, to guard against unintended overrides, I wonder if we should check whether the git repository for the embedded application exists and warn on git repo content getting overwritten before applying the override?

nimakaviani · 2024-04-18T04:11:26Z

pkg/cmd/create/root.go

+	_, ok := corePkgs[name]
+	if !ok {
+		return v1alpha1.PackageCustomization{}, fmt.Errorf("customization for %s not supported", name)
+	}


Suggested change

_, ok := corePkgs[name]

if !ok {

return v1alpha1.PackageCustomization{}, fmt.Errorf("customization for %s not supported", name)

}

if _, ok := corePkgs[name]; !ok {

return v1alpha1.PackageCustomization{}, fmt.Errorf("customization for %s not supported", name)

}

nimakaviani · 2024-04-18T04:12:07Z

pkg/cmd/create/root.go

+	err = helpers.ValidateKubernetesYamlFile(paths[0])
+	if err != nil {
+		return v1alpha1.PackageCustomization{}, err
+	}


Suggested change

err = helpers.ValidateKubernetesYamlFile(paths[0])

if err != nil {

return v1alpha1.PackageCustomization{}, err

}

if err = helpers.ValidateKubernetesYamlFile(paths[0]); err != nil {

return v1alpha1.PackageCustomization{}, err

}

nimakaviani · 2024-04-18T04:22:54Z

pkg/cmd/create/root.go

+	name := s[0]
+	_, ok := corePkgs[name]
+	if !ok {
+		return v1alpha1.PackageCustomization{}, fmt.Errorf("customization for %s not supported", name)


this logic deserves to be pulled out into a reusable bit

nimakaviani · 2024-04-18T04:26:51Z

pkg/k8s/deserialize.go

+			o, ok := overrideMap[id]
+			if ok {


can be inlined

nabuskey force-pushed the custom-core-packages branch from 9f79b03 to 23c9a31 Compare March 21, 2024 17:59

nabuskey marked this pull request as ready for review March 21, 2024 18:10

nabuskey changed the title ~~WIP: Custom core packages~~ custom core packages Mar 21, 2024

nabuskey requested review from nimakaviani, greghaynes and jessesanford March 21, 2024 18:15

nabuskey added 3 commits April 17, 2024 21:50

allow for core package customizations

8f05985

Signed-off-by: Manabu McCloskey <[email protected]>

reconcile core packages every time

8cb649d

Signed-off-by: Manabu McCloskey <[email protected]>

rebase to main

b0faa57

Signed-off-by: Manabu McCloskey <[email protected]>

nabuskey force-pushed the custom-core-packages branch from 64ae7c3 to b0faa57 Compare April 17, 2024 21:54

nimakaviani approved these changes Apr 18, 2024

View reviewed changes

nabuskey merged commit fd4bd17 into cnoe-io:main Apr 18, 2024
2 checks passed

nabuskey deleted the custom-core-packages branch April 24, 2024 00:01

nabuskey mentioned this pull request Jun 17, 2024

ensure core package customizations are processed #307

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

custom core packages #180

custom core packages #180

nabuskey commented Mar 19, 2024 •

edited

Loading

jessesanford commented Mar 20, 2024 •

edited

Loading

nabuskey commented Mar 21, 2024

nimakaviani commented Mar 26, 2024 •

edited

Loading

nimakaviani commented Apr 2, 2024

nabuskey commented Apr 11, 2024

cmoulliard commented Apr 12, 2024

cmoulliard commented Apr 12, 2024

cmoulliard commented Apr 12, 2024

nabuskey commented Apr 12, 2024 •

edited

Loading

cmoulliard commented Apr 15, 2024

nimakaviani commented Apr 16, 2024

cmoulliard commented Apr 16, 2024

nabuskey commented Apr 16, 2024

nabuskey commented Apr 17, 2024 •

edited

Loading

nimakaviani left a comment

nimakaviani Apr 18, 2024

nimakaviani Apr 18, 2024

nimakaviani Apr 18, 2024

nimakaviani Apr 18, 2024

custom core packages #180

custom core packages #180

Conversation

nabuskey commented Mar 19, 2024 • edited Loading

jessesanford commented Mar 20, 2024 • edited Loading

nabuskey commented Mar 21, 2024

nimakaviani commented Mar 26, 2024 • edited Loading

nimakaviani commented Apr 2, 2024

nabuskey commented Apr 11, 2024

cmoulliard commented Apr 12, 2024

cmoulliard commented Apr 12, 2024

cmoulliard commented Apr 12, 2024

nabuskey commented Apr 12, 2024 • edited Loading

cmoulliard commented Apr 15, 2024

nimakaviani commented Apr 16, 2024

cmoulliard commented Apr 16, 2024

nabuskey commented Apr 16, 2024

nabuskey commented Apr 17, 2024 • edited Loading

nimakaviani left a comment

Choose a reason for hiding this comment

nimakaviani Apr 18, 2024

Choose a reason for hiding this comment

nimakaviani Apr 18, 2024

Choose a reason for hiding this comment

nimakaviani Apr 18, 2024

Choose a reason for hiding this comment

nimakaviani Apr 18, 2024

Choose a reason for hiding this comment

nabuskey commented Mar 19, 2024 •

edited

Loading

jessesanford commented Mar 20, 2024 •

edited

Loading

nimakaviani commented Mar 26, 2024 •

edited

Loading

nabuskey commented Apr 12, 2024 •

edited

Loading

nabuskey commented Apr 17, 2024 •

edited

Loading