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

🐛 Fix secret selection logic for ownerRef test #7973

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 30 additions & 2 deletions cmd/clusterctl/client/cluster/ownergraph.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ import (
"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"

clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
)

// OwnerGraph contains a graph with all the objects considered by clusterctl move as nodes and the OwnerReference relationship
Expand Down Expand Up @@ -50,15 +53,20 @@ func nodeToOwnerRef(n *node, attributes ownerReferenceAttributes) metav1.OwnerRe

// GetOwnerGraph returns a graph with all the objects considered by clusterctl move as nodes and the OwnerReference relationship between those objects as edges.
// NOTE: this data structure is exposed to allow implementation of E2E tests verifying that CAPI can properly rebuild its
// own owner references; there is no guarantee about the stability of this API.
// own owner references; there is no guarantee about the stability of this API. Using this test with providers may require
// a custom implementation of this function, or the OwnerGraph it returns.
func GetOwnerGraph(namespace, kubeconfigPath string) (OwnerGraph, error) {
p := newProxy(Kubeconfig{Path: kubeconfigPath, Context: ""})
invClient := newInventoryClient(p, nil)

graph := newObjectGraph(p, invClient)

cl, err := p.NewClient()
if err != nil {
return OwnerGraph{}, errors.Wrap(err, "failed to create client for ownerGraph")
}
// Gets all the types defined by the CRDs installed by clusterctl plus the ConfigMap/Secret core types.
err := graph.getDiscoveryTypes()
err = graph.getDiscoveryTypes()
if err != nil {
return OwnerGraph{}, errors.Wrap(err, "failed to retrieve discovery types")
}
Expand All @@ -71,6 +79,17 @@ func GetOwnerGraph(namespace, kubeconfigPath string) (OwnerGraph, error) {
}
owners := OwnerGraph{}
for _, v := range graph.uidToNode {
// The Discovery function returns all secrets in the Cluster namespace. Ensure a Secret that is not part of the
// Cluster is not added to the OwnerGraph.
if v.identity.Kind == "Secret" {
sbueringer marked this conversation as resolved.
Show resolved Hide resolved
clusterSecret, err := isClusterSecret(v.identity, cl)
if err != nil {
return OwnerGraph{}, err
}
if !clusterSecret {
continue
}
}
n := OwnerGraphNode{Object: v.identity, Owners: []metav1.OwnerReference{}}
for owner, attributes := range v.owners {
n.Owners = append(n.Owners, nodeToOwnerRef(owner, attributes))
Expand All @@ -79,3 +98,12 @@ func GetOwnerGraph(namespace, kubeconfigPath string) (OwnerGraph, error) {
}
return owners, nil
}

// isClusterSecret checks whether a Secret is related to a CAPI Cluster by checking if the secret type is ClusterSecretType.
func isClusterSecret(ref corev1.ObjectReference, c client.Client) (bool, error) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've tested this locally with Kubernetes 1.20.15 and it works. I think this approach makes sense for what we're currently testing, but we might want to be careful of expectations if providers pick up this test.

s := &corev1.Secret{}
if err := c.Get(ctx, client.ObjectKey{Namespace: ref.Namespace, Name: ref.Name}, s); err != nil {
return false, err
}
return s.Type == clusterv1.ClusterSecretType, nil
}
2 changes: 1 addition & 1 deletion util/secret/secret.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func Name(cluster string, suffix Purpose) string {
return fmt.Sprintf("%s-%s", cluster, suffix)
}

// ParseSecretName return the cluster name and the suffix Purpose in name is a valid cluster secrets,
// ParseSecretName return the cluster name and the suffix Purpose in name is a valid cluster secret,
// otherwise it return error.
func ParseSecretName(name string) (string, Purpose, error) {
separatorPos := strings.LastIndex(name, "-")
Expand Down