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

Hybrid is missing config for cert-manager and webhooks #188

Closed
camilamacedo86 opened this issue Aug 3, 2022 · 4 comments · Fixed by #191
Closed

Hybrid is missing config for cert-manager and webhooks #188

camilamacedo86 opened this issue Aug 3, 2022 · 4 comments · Fixed by #191
Assignees

Comments

@camilamacedo86
Copy link
Contributor

camilamacedo86 commented Aug 3, 2022

Description

If we check in the config/default/kustomization.yaml, Hybrid provides the cert-manager and webhooks config but it is missing the specific directories.

Propose Solution

a) Add missing scaffolds

If we should support both then we need to ensure the scaffolds in the init, also we would need to add the flags and implement the create api subcommand. See:

The default scaffolds are:

OR

b) Remove options from config/default/kustomization.yaml (such as we do for helm)

We need to change the kustomize manifest to replace these points such as we do for the helm, see: https://github.com/operator-framework/operator-sdk/blob/master/testdata/helm/memcached-operator/config/default/kustomization.yaml

@camilamacedo86
Copy link
Contributor Author

c/c @varshaprasad96

@laxmikantbpandhare
Copy link
Member

laxmikantbpandhare commented Aug 8, 2022

I will take a look into it.

@laxmikantbpandhare laxmikantbpandhare self-assigned this Aug 8, 2022
@laxmikantbpandhare
Copy link
Member

@camilamacedo86 - I am able to recreate this and I agree with you. So either we need to add logic for adding these directories webhook and cert-manager with respective files or remove those commented code from scaffolding.

@varshaprasad96
Copy link
Member

varshaprasad96 commented Aug 8, 2022

@camilamacedo86 The issue seems to be that for the sake of project to be compatible with Helm api, we are removing those files. But eventually its getting added due to Go. The solution for this would be to entirely remove the Webhooks/Certmanager part from config/default/kustomization.yaml as it is with plain Helm: https://github.com/operator-framework/operator-sdk/blob/master/testdata/helm/memcached-operator/config/default/kustomization.yaml. We would document it explicitly that Hybrid-Helm has not been tested against webhooks and hence there is no scaffolding for that. We will create a new issue to track that feature and test to add it with the right Go API. I would expect this logic to be a part of edit command just for this plugin.

cc: @laxmikantbpandhare

@jmrodri @joelanford any objections?

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 a pull request may close this issue.

3 participants