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

[Bug]: issue CRD secrets keyVault #785

Open
1 task done
DrummyFloyd opened this issue Jul 21, 2024 · 7 comments
Open
1 task done

[Bug]: issue CRD secrets keyVault #785

DrummyFloyd opened this issue Jul 21, 2024 · 7 comments
Labels
bug Something isn't working needs:triage

Comments

@DrummyFloyd
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Affected Resource(s)

apiVersion: keyvault.azure.upbound.io/v1beta1
kind: Secret

Resource MRs required to reproduce the bug

apiVersion: keyvault.azure.upbound.io/v1beta1
kind: Secret

Steps to Reproduce

---
apiVersion: pkg.crossplane.io/v1
kind: Provider
metadata:
  name: provider-azure-keyvault
spec:
  package: xpkg.upbound.io/upbound/provider-azure-keyvault:v1.4.0
---
apiVersion: keyvault.azure.upbound.io/v1beta1
kind: Secret
metadata:
  name: test-secret
spec:
  forProvider:
    keyVaultIdSelector:
      matchLabels:
        xvault.azure.xor.io/name: "vault-1245azer8907"
    valueSecretRef:
      key: test
      name: test
      namespace: crossplane
---
# https://kubernetes.io/docs/concepts/configuration/secret/
apiVersion: v1
kind: Secret
metadata:
  name: mysecret
type: Opaque
data:
  test: test

What happened?

secret/mysecret unchanged
The Secret "test-secret" is invalid: 
* spec: Required value
* <nil>: Invalid value: "null": some validation rules were not checked because the object was invalid; correct the existing errors to complete validation

can't achieve to get the crd read correctly crd

❯ k get crds |rg azure
accesspolicies.keyvault.azure.upbound.io                             2024-07-19T15:01:08Z
certificatecontacts.keyvault.azure.upbound.io                        2024-07-19T15:01:08Z
certificateissuers.keyvault.azure.upbound.io                         2024-07-19T15:01:08Z
certificates.keyvault.azure.upbound.io                               2024-07-19T15:01:08Z
keys.keyvault.azure.upbound.io                                       2024-07-19T15:01:08Z
managedhardwaresecuritymodules.keyvault.azure.upbound.io             2024-07-19T15:01:08Z
managedstorageaccounts.keyvault.azure.upbound.io                     2024-07-19T15:01:08Z
managedstorageaccountsastokendefinitions.keyvault.azure.upbound.io   2024-07-19T15:01:08Z
providerconfigs.azure.upbound.io                                     2024-07-19T15:01:06Z
providerconfigusages.azure.upbound.io                                2024-07-19T15:01:06Z
resourcegroups.azure.upbound.io                                      2024-07-19T15:01:06Z
resourceproviderregistrations.azure.upbound.io                       2024-07-19T15:01:06Z
secrets.keyvault.azure.upbound.io                                    2024-07-19T15:01:08Z => present 
storeconfigs.azure.upbound.io                                        2024-07-19T15:01:06Z
subscriptions.azure.upbound.io                                       2024-07-19T15:01:06Z
vaults.keyvault.azure.upbound.io                                     2024-07-19T15:01:08Z
xvaults.azure.xor.io    

Relevant Error Output Snippet

No response

Crossplane Version

1.16

Provider Version

1.4

Kubernetes Version

1.28

Kubernetes Distribution

OVH

Additional Info

No response

@DrummyFloyd DrummyFloyd added bug Something isn't working needs:triage labels Jul 21, 2024
@naimadswdn
Copy link

naimadswdn commented Aug 30, 2024

The issue is related to the spec.initProvider.valueSecretRef started to being required with the 1.3.0 release.

I just tried to update the provider version from 1.2.0 to 1.5.0 and I am suffering such an error:
spec.initProvider.valueSecretRef: Required value, <nil>: Invalid value: "null": some validation rules were not checked because the object was invalid; correct the existing errors to complete validation

For the 1.2.0 provider it does works as expected.
It is even visible in the docs:

I see it has been added by the @gravufo on the 1st of June: https://github.com/crossplane-contrib/provider-upjet-azure/blame/main/package/crds/keyvault.azure.upbound.io_secrets.yaml#L321

Is the fix so simple as removing the:

required:
- valueSecretRef

from the CRD definition?
If yes, then I can create a PR. Otherwise, I am not into golang to dig into that :(

EDIT: For the newly created resources it does works, probably because the LateInitialize - it only stops to work with already existing resources.
Of course, I could add this value (spec.initProvider.valueSecretRef) in order to finish the provider update, but its clearly an unwanted behavior.

@gravufo
Copy link
Contributor

gravufo commented Sep 2, 2024

@naimadswdn thanks for tagging me. I checked my changes and I don't think I voluntarily introduced this. In fact, all of these files are auto-generated and the changes I have done in that PR should not have affected this. My guess is that the generator has been modified somehow between the last time it was run and the time I did my PR, so I ended up having those changes at the same time.
Unfortunately, removing the line from the CRD will not be a permanent solution. Any subsequent PR that runs make generate will reintroduce the issue.

I think someone more familiar with Upjet may have a better idea of where to do a fix.

@naimadswdn
Copy link

Thanks @gravufo for the meaningful response. 👍
I see that there were changes in the build submodule on the 30th of May, so maybe @turkenf can help us to understand what happen here.

@naimadswdn
Copy link

Any response about it?
I would like to understand why one of the initProvider (that is still in the beta!!) fields become Required.

The issue can be easily reproduced.
First create a new Secret object with provider 1.2.0.
Then, update the provider to higher version (whatever, it may be currently highest 1.5.1):
The Secret is not going to be Synced, because of missing initProvider.valueSecretRef.

@CCOLLOT
Copy link

CCOLLOT commented Nov 5, 2024

Any news? This issue is making the provider hard to use

@DrummyFloyd
Copy link
Author

Any news? This issue is making the provider hard to use

tbh honest on my side, i just created a css & a PushSecret (external secret operator) to push the secret directly to vault

@gravufo
Copy link
Contributor

gravufo commented Nov 6, 2024

There's an upjet meeting today, you may want to bring this issue up. Here is the google docs agenda: https://docs.google.com/document/d/1QJfsAk-pdo_j2cKsJtG6W-BOQh_3ilhiRVTYVIGkDXM/edit?tab=t.0#heading=h.cfsfte4lw0gh

The zoom link: https://upbound-io.zoom.us/j/82277438090?pwd=pgJE4lGa1KkXyZZ80N6GwvHNZhuave.1r

The call takes place at 08:00 PT/16:00 UTC.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs:triage
Projects
None yet
Development

No branches or pull requests

4 participants