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

Revisit configuration options #2003

Closed
lburgazzoli opened this issue Feb 4, 2021 · 10 comments · Fixed by #2355
Closed

Revisit configuration options #2003

lburgazzoli opened this issue Feb 4, 2021 · 10 comments · Fixed by #2355
Assignees
Labels
area/core Core features of the integration platform area/ux Improves user experience
Milestone

Comments

@lburgazzoli
Copy link
Contributor

As today there is a little bit of confusion about the options to configure the runtime (you can blame me as I introduced them in the early days of camel-k) and I think it is now time to rethink them.

I'm thinking about something like:

--build-property  key=val 
--config-property key=val[@build]
--config          configmap|secret|file:name[/key]
--resource        configmap|secret|file:name[/key][@path]

This issue also relates to:

@lburgazzoli
Copy link
Contributor Author

/cc @astefanutti @squakez

@lburgazzoli lburgazzoli added area/core Core features of the integration platform area/ux Improves user experience labels Feb 4, 2021
@astefanutti astefanutti changed the title Revisit configuration oprtions Revisit configuration options Feb 5, 2021
@nicolaferraro nicolaferraro added this to the 1.4.0 milestone Feb 9, 2021
@astefanutti
Copy link
Member

I realise from apache/camel-k-runtime#624, that any ConfigMap or Secret is (attempted to be) loaded as properties file.

I'd agree with @lburgazzoli comment from apache/camel-k-runtime#593:

I think the main issue is that we don't have a way to distinguish between configmaps and secrets used to store properties vs resources.

It seems only application properties files should be mounted into the conf.d directory, while other ConfigMaps and Secrets should be mounted on the side. IIUIC, this is what would be for the --config and --resource options for.

So currently, we have the ServiceBindings, ConfigMaps and Secrets mounted under the conf.d directory. Only the ConfigMaps and the Secrets added with the new --config option should be mounted there. The one added with the existing --resource should be mounted in etc/camel/resources.

That also means the existing --configmap and --secret options are removed.

@astefanutti astefanutti self-assigned this Feb 11, 2021
@lburgazzoli
Copy link
Contributor Author

Yep, I'd also add a new property function on camel-kruntime like resource:name/key to ease the process of loading resources.

@heiko-braun
Copy link

Would a configmap resource contain build-properties and config-properties itself?

@astefanutti astefanutti removed their assignment May 10, 2021
@squakez
Copy link
Contributor

squakez commented May 14, 2021

I am working on this.

squakez added a commit to squakez/camel-k that referenced this issue May 14, 2021
squakez added a commit to squakez/camel-k that referenced this issue May 14, 2021
* Changed Java test with Groovy test
* Covered all resources cases (base encoded)

Ref apache#2003
squakez added a commit to squakez/camel-k that referenced this issue May 14, 2021
squakez added a commit to squakez/camel-k that referenced this issue May 14, 2021
* Added configmap and secret integration tests
* Refactored to be included and run under /common dir

Ref apache#2003
squakez added a commit to squakez/camel-k that referenced this issue May 17, 2021
* Added configmap and secret integration tests
* Refactored to be included and run under /common dir

Ref apache#2003
astefanutti pushed a commit that referenced this issue May 17, 2021
* Changed Java test with Groovy test
* Covered all resources cases (base encoded)

Ref #2003
astefanutti pushed a commit that referenced this issue May 17, 2021
astefanutti pushed a commit that referenced this issue May 17, 2021
* Added configmap and secret integration tests
* Refactored to be included and run under /common dir

Ref #2003
@lburgazzoli
Copy link
Contributor Author

lburgazzoli commented May 19, 2021

@squakez @nicolaferraro @astefanutti

wondering if we should create a configuration trait at this point so when we will implement #2165, we could also use the same mechanic to attach configuration properties to bindings, kamelets & what not

@astefanutti
Copy link
Member

That sound like a good idea worth exploring. If we follow that path, we may want to deprecate the .spec.configuration field from the Integration API/CRD.

@astefanutti
Copy link
Member

astefanutti commented May 19, 2021

Also, in that spirit of moving / "encapsulating" the configurability responsibility to the traits, it may be logical to:

  • Move the .spec.repositories field (Maven repositories) into the builder trait
  • Move the .spec.serviceAccountName field into the pod trait

@squakez
Copy link
Contributor

squakez commented May 20, 2021

@squakez @nicolaferraro @astefanutti

wondering if we should create a configuration trait at this point so when we will implement #2165, we could also use the same mechanic to attach configuration properties to bindings, kamelets & what not

Yes, that seems the best path to follow. As we must ensure compatibility, I may follow an approach where I will develop the feature minimizing the impact on the existing code, if I see it makes sense, and then encapsulating the logic into a separate trait.

squakez added a commit to squakez/camel-k that referenced this issue Jun 16, 2021
* Provide support for configmap, secret or local file
* Refactoring the structure of mount points to distinguish clearly between config and resource options

Ref apache#2003
squakez added a commit to squakez/camel-k that referenced this issue Jun 16, 2021
squakez added a commit to squakez/camel-k that referenced this issue Jun 16, 2021
squakez added a commit to squakez/camel-k that referenced this issue Jun 16, 2021
Adding backward compatibility and a deprecation notice.

Ref apache#2003
squakez added a commit to squakez/camel-k that referenced this issue Jun 16, 2021
squakez added a commit to squakez/camel-k that referenced this issue Jun 16, 2021
Adding the possibility to specify the destination path for any resource

Ref apache#2003
squakez added a commit to squakez/camel-k that referenced this issue Jun 16, 2021
squakez added a commit to squakez/camel-k that referenced this issue Jun 16, 2021
Won't accept any resource/config file larger than 1MB, due to the limitation on K8S CustomResources

Ref apache#2003
squakez added a commit to squakez/camel-k that referenced this issue Jun 16, 2021
squakez added a commit to squakez/camel-k that referenced this issue Jun 16, 2021
squakez added a commit to squakez/camel-k that referenced this issue Jun 16, 2021
squakez added a commit to squakez/camel-k that referenced this issue Jun 16, 2021
* Added support to specify a single key from a configmap/secret in --resource or --config flag
* Refactoring the RunConfigOption struct to include the new feature and hide the complexity to usage

Ref apache#2003
squakez added a commit to squakez/camel-k that referenced this issue Jun 16, 2021
squakez added a commit to squakez/camel-k that referenced this issue Jun 16, 2021
squakez added a commit to squakez/camel-k that referenced this issue Jun 16, 2021
squakez added a commit to squakez/camel-k that referenced this issue Jun 16, 2021
squakez added a commit to jboss-fuse/camel-k that referenced this issue Jun 16, 2021
* feat(cmd/run): resource option refactoring

* Provide support for configmap, secret or local file
* Refactoring the structure of mount points to distinguish clearly between config and resource options

Ref apache#2003

* refactor(crd): configuration resource type

Ref apache#2003

* refactor(example): resource run option

* refactor(e2e): resource option integration test

Ref apache#2003

* feat(cmd/run): --resource filename compatibility

Adding backward compatibility and a deprecation notice.

Ref apache#2003

* feat(cmd/run): modeline resource support

Ref apache#2003

* feat(cmd/run): resource @path support

Adding the possibility to specify the destination path for any resource

Ref apache#2003

* chore(crd): autogen update

Ref apache#2003

* doc(example): resource @path

* feat(cmd/run): file size limitation

Won't accept any resource/config file larger than 1MB, due to the limitation on K8S CustomResources

Ref apache#2003

* refactor(cmd/run): synch all option files

Ref apache#2003

* chore(e2e): run option with destination path

Ref apache#2003

* feat(cmd/run): validate destination path

Ref apache#2003

* fix(e2e): test name typo

* feat(cmd/run): configmap/secret key filtering

* Added support to specify a single key from a configmap/secret in --resource or --config flag
* Refactoring the RunConfigOption struct to include the new feature and hide the complexity to usage

Ref apache#2003

* chore(crd): configmap/secret eky filtering regen

Ref apache#2003

* chore(e2e): configmap/secret key test

Ref apache#2003

* doc(example): configmap/secret key selection

* feat(cmd/run): warn when using path on --config

Ref apache#2003

* chore(cmd/run): single resource dir and cli syntax description

Ref apache#2003

* feat(cmd/run): resource option refactoring

Go mod vendor synch
squakez added a commit to squakez/camel-k that referenced this issue Jun 16, 2021
nicolaferraro pushed a commit that referenced this issue Jun 16, 2021
squakez added a commit to squakez/camel-k that referenced this issue Jun 16, 2021
squakez added a commit to jboss-fuse/camel-k that referenced this issue Jun 16, 2021
* doc: new configuration explained

Ref apache#2003

* fix(doc): kube-config global flag

* doc(examples): http

* doc(examples): databases

* chore(examples): fix with resource dir
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/core Core features of the integration platform area/ux Improves user experience
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants