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

feat(trait): leverage ConfigMap binary data for resources #1965

Merged
merged 6 commits into from
Feb 3, 2021

Conversation

squakez
Copy link
Contributor

@squakez squakez commented Feb 1, 2021

With this PR we include the possibility to use BinaryData in ConfigMaps, leaving any encoding/decoding to the k8s platform. The decision of the using BinaryData vs Data is based on the sniffed mime-type of the resource passed. In order to maintain the possibility to explicit encode we have the following situations:

  1. kamel run ... --resource file.text --> will leverage Data ConfigMap, mounting a volume with a plain text data file.
  2. kamel run ... --resource file.binary-->will leverage BinaryData ConfigMap, mounting a volume with a binary data file.
  3. kamel run ... --resource file.text/file.binary --compression --> will leverage Data ConfigMap, we base64 encode the content, mounting a volume with a base64 encoded file. Decoding will be up to the application logic.

Close #1946, fix #1881

Release Note

feat(trait): leverage ConfigMap binary data for resources

Copy link
Member

@astefanutti astefanutti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! I think the CRDs have to be regenerated to take the API update into account.

@squakez
Copy link
Contributor Author

squakez commented Feb 1, 2021

LGTM! I think the CRDs have to be regenerated to take the API update into account.

Do you mean this, right? what's the process to update them?

@astefanutti
Copy link
Member

LGTM! I think the CRDs have to be regenerated to take the API update into account.

Do you mean this, right? what's the process to update them?

Exactly. You can run make generate-crd.

Also running make generate-json-schema just to be sure.

@astefanutti
Copy link
Member

@squakez it seems like there is a conflict with master branch.

@squakez
Copy link
Contributor Author

squakez commented Feb 3, 2021

@squakez it seems like there is a conflict with master branch.

I'll rebase

* Any binary data will be using BinaryData ConfiMap instead of Data. We let the cluster to encode/decode the resource
* Any text resource will be still using the Data (plain) ConfigMap
* The `compression` feature can be run both on binary and text resources, providing a base64 encoded file.
* Added unit test to check all the possible scenarios

Close apache#1946, close apache#1881
With apache#1946 we are no longer needing a special `compress-binary` flag
Include rawContent and contentType for resources

Ref apache#1946
@squakez
Copy link
Contributor Author

squakez commented Feb 3, 2021

@astefanutti it should be clear now

@astefanutti
Copy link
Member

Thanks @squakez!

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

Successfully merging this pull request may close these issues.

Usage of binary data in ConfigMaps Corrupted binaries attached as resource
3 participants