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

The YAML tab displays a wrong version of CRD #3626

Closed
chanwit opened this issue Apr 14, 2023 · 14 comments · Fixed by #3980
Closed

The YAML tab displays a wrong version of CRD #3626

chanwit opened this issue Apr 14, 2023 · 14 comments · Fixed by #3980
Assignees
Labels
area/ui Issues that require front-end work bug Something isn't working severity/high low < medium < high < critical team/tangerine

Comments

@chanwit
Copy link
Member

chanwit commented Apr 14, 2023

Problem

According to this:

func DefaultPrimaryKinds() (*PrimaryKinds, error) {
kinds := New()
scheme, err := kube.CreateScheme()
if err != nil {
return nil, err
}
for gvk := range scheme.AllKnownTypes() {
kinds.kinds[gvk.Kind] = gvk
}
return kinds, nil
}

The DefaultPrimaryKinds() function in the provided code snippet is designed to loop over all known types in the Kubernetes scheme and create a map of GroupVersionKinds (GVKs). However, there is a hidden bug in this function, which makes it unable to obtain the storage version from the scheme.

The issue arises because the function relies on the scheme.AllKnownTypes() map, which contains types registered in the program during compile time. If the program uses a different version of an object (e.g., v1beta2 version of GitRepository) than the environment it's running in (e.g., an environment with the v1 version of GitRepository), the program does not have the knowledge of the environment's version (in this example, GitRepository/v1). This is because the environment's version only exists in the actual cluster, not in the program's registered types.

As a result of this bug, the YAML tab might display incorrect versions of objects, leading to potential issues and inconsistencies when working with Kubernetes objects.

Proposed solution

To address the bug, the first approach would be to update the program to have matching versions of objects registered, ensuring consistency between the program and the environment it's running in. By doing this, the program will have knowledge of the correct versions and can display accurate information in the YAML tab.

If upgrading the program to have matched versions registered does not resolve the issue effectively, a follow-up ticket will be created to further investigate the problem and find an alternative solution. This ensures that the bug is addressed and the correct versions of objects are displayed, improving the overall user experience and preventing potential issues when working with Kubernetes objects.

@chanwit chanwit added bug Something isn't working severity/low low < medium < high < critical team/denim area/ui Issues that require front-end work labels Apr 14, 2023
@joshri
Copy link
Contributor

joshri commented Apr 14, 2023

SO -

My podinfo GitRepo source has matching apiVersion and annotation:
image.png

But my helloworld GitRepo source (which is from the tf-controller getting started guide) does not:
image.png

When I get the yaml in a terminal, it also doesn't match:
image.png

Which makes me think this isn't something that's happening on the frontend.

Everything else on my cluster (Helm charts, repos, kustomizations, buckets, and non-flux-primitives) match. Couldn't find another example of this.

@katya-makarevich katya-makarevich assigned chanwit and unassigned joshri Apr 18, 2023
@chanwit
Copy link
Member Author

chanwit commented Apr 18, 2023

I'm talking to Kevin and probably Simon to investigate this.

@chanwit
Copy link
Member Author

chanwit commented Apr 19, 2023

Found a feasible solution. I'll go fix it this way first.

@squaremo
Copy link
Contributor

kubectl gives you the preferred version (usually the stored version); so if I create a GitRepository marked as apiVersion: source.toolkit.fluxcd.io/v1beta2, it accepts it, and when I kubectl edit it, I get a v1 file to edit. I think the same approach would work here: ask the Kubernetes API server what the preferred version is, and use that.

@foot
Copy link
Contributor

foot commented Jul 19, 2023

@bigkevmcd noted we could check out kubectl does it

@chanwit
Copy link
Member Author

chanwit commented Jul 19, 2023

I don't think we can fix the apiVersion under the YAML tab on the resource details page. disappointed

Summary:
go-client does not care about GroupVersion only the Kind. On Get and List, the requested object's gvk will be set to be the same as the request was.

If we request v1beta2 we will get a v1beta2, if we request a v1 we will get a v1 with the same object. If we iterate over all known group versions, we will get a response on all of them with the same objects. If we request only v1 (as it does not matter) it will show v1 even if the object is v1beta2.

My assumption on the reason:
You can't have multiple resources with the same Kind, Namespace, and Name, does not matter the apiVersion.

I went down to a deep rabbit hole with controller-runtime and stopped when I was in the source code of client-go about handling metadata lists. My assumption (and it matches with our experiences) is, the list query does not care about group versions at all.

For example controller-runtime sets the object's gvk to the requested gvk on list.
It does the same for get.

The finding by @yitsushi #3811 (comment)

@bigkevmcd
Copy link
Contributor

Ultimately, it depends on what you think you're showing, if you think you're showing the version in the cluster that's different from the version that might be in git.

Things like storage versions and conversion webhooks make what's in git somewhat irrelevant, git is surprisingly, not the source of truth for this.

@enekofb
Copy link
Contributor

enekofb commented Aug 7, 2023

@chanwit is there anything that we think we would do around this ticket?

@chanwit
Copy link
Member Author

chanwit commented Aug 7, 2023

I think we could close this issue for now, as the issue would be fixed already by our Flux v2 upgrade.

If the problem still persists, we can reopen this issue.

@enekofb
Copy link
Contributor

enekofb commented Aug 8, 2023

Based on our findings yesterday, it seems that we should revisit this logic for the GitRepository case where we are not seeing predictable resolution to v1 apiGroup

https://github.com/weaveworks/weave-gitops/blob/main/core/server/primarykinds.go#L126

Scenario #1 - both gvk exists

2023-08-08T07:27:06.856Z	INFO	gitops	server/primarykinds.go:132	AllKnownTypes getPrimaryKinds	{"gvk": "source.toolkit.fluxcd.io/v1beta2, Kind=GitRepository"}
...
2023-08-08T07:27:06.857Z	INFO	gitops	server/primarykinds.go:132	AllKnownTypes getPrimaryKinds	{"gvk": "source.toolkit.fluxcd.io/v1, Kind=GitRepository"}

Scenario #2 - only v1 exists

2023-08-08T07:31:09.845Z	INFO	gitops	server/primarykinds.go:132	AllKnownTypes getPrimaryKinds	{"gvk": "source.toolkit.fluxcd.io/v1, Kind=GitRepositoryList"}
2023-08-08T07:31:09.845Z	INFO	gitops	server/primarykinds.go:132	AllKnownTypes getPrimaryKinds	{"gvk": "/v1, Kind=Secret"}
2023-08-08T07:31:09.845Z	INFO	gitops	server/primarykinds.go:132	AllKnownTypes getPrimaryKinds	{"gvk": "notification.toolkit.fluxcd.io/v1beta2, Kind=PatchOptions"}
2023-08-08T07:31:09.845Z	INFO	gitops	server/primarykinds.go:132	AllKnownTypes getPrimaryKinds	{"gvk": "source.toolkit.fluxcd.io/v1, Kind=GitRepository"}
2023-08-08T07:31:09.845Z	INFO	gitops	server/primarykinds.go:132	AllKnownTypes getPrimaryKinds	{"gvk": "kustomize.toolkit.fluxcd.io/v1, Kind=CreateOptions"}

We have also seen Scenario #3 - only beta exits

@enekofb enekofb added severity/high low < medium < high < critical and removed severity/low low < medium < high < critical labels Aug 8, 2023
@enekofb
Copy link
Contributor

enekofb commented Aug 8, 2023

bumping the severity as from v0.29 we want to manage v1 resources over beta

@lasomethingsomething
Copy link
Contributor

@enekofb I see that you self-assigned this. Should I drop @chanwit and Team Wild W from this issue?

@enekofb
Copy link
Contributor

enekofb commented Aug 15, 2023

Yes @LappleApple i take over

@enekofb
Copy link
Contributor

enekofb commented Sep 5, 2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ui Issues that require front-end work bug Something isn't working severity/high low < medium < high < critical team/tangerine
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants