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

nil pointer dereference when using a wrong selector #260

Closed
prometherion opened this issue Jan 5, 2024 · 1 comment
Closed

nil pointer dereference when using a wrong selector #260

prometherion opened this issue Jan 5, 2024 · 1 comment
Assignees
Milestone

Comments

@prometherion
Copy link
Contributor

Defining a Profile's or ClusterProfile's selector with a wrong format, the Addon Controller crashes with the following stacktrace:

panic: runtime error: invalid memory address or nil pointer dereference [recovered]
        panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x30 pc=0x17a1a6e]

goroutine 359 [running]:
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Reconcile.func1()
        /go/pkg/mod/sigs.k8s.io/controller-runtime@v0.16.3/pkg/internal/controller/controller.go:116 +0x1e5
panic({0x1e94e80?, 0x36760c0?})
        /usr/local/go/src/runtime/panic.go:914 +0x21f
github.com/projectsveltos/libsveltos/lib/clusterproxy.getMatchingCAPIClusters({0x24ef410, 0xc0017b2c00}, {0x24fe340?, 0xc0005c0900?}, {0x0, 0x0}, {0xc0013023c0, 0x1a}, {{0x24f42e8, 0xc0011fb400}, ...})
        /go/pkg/mod/github.com/projectsveltos/libsveltos@v0.21.0/lib/clusterproxy/cluster_utils.go:421 +0x38e
github.com/projectsveltos/libsveltos/lib/clusterproxy.GetMatchingClusters({0x24ef410, 0xc0017b2c00}, {0x24fe340, 0xc0005c0900}, {0x0, 0x0}, {0xc0013023c0, 0x1a}, {{0x24f42e8, 0xc0011fb400}, ...})
        /go/pkg/mod/github.com/projectsveltos/libsveltos@v0.21.0/lib/clusterproxy/cluster_utils.go:478 +0x85
github.com/projectsveltos/addon-controller/controllers.getMatchingClusters({0x24ef410, 0xc0017b2c00}, {0x24fe340, 0xc0005c0900}, {0xc0013023c0, 0x1a}, 0x21?, {{0x24f42e8, 0xc0011fb400}, 0x0})
        /workspace/controllers/profile_utils.go:57 +0xf1
github.com/projectsveltos/addon-controller/controllers.(*ProfileReconciler).reconcileNormal(0xc000400140, {0x24ef410, 0xc0017b2c00}, 0xc00055be00)
        /workspace/controllers/profile_controller.go:185 +0x1d0
github.com/projectsveltos/addon-controller/controllers.(*ProfileReconciler).Reconcile(0xc000400140, {0x24ef410?, 0xc0017b2c00?}, {{{0xc0013023c0, 0x1a}, {0xc0017aead6, 0x6}}})
        /workspace/controllers/profile_controller.go:150 +0x5bf
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Reconcile(0x24f42e8?, {0x24ef410?, 0xc0017b2c00?}, {{{0xc0013023c0?, 0xb?}, {0xc0017aead6?, 0x0?}}})
        /go/pkg/mod/sigs.k8s.io/controller-runtime@v0.16.3/pkg/internal/controller/controller.go:119 +0xb7
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler(0xc0006a4280, {0x24ef448, 0xc0005d72c0}, {0x1f78980?, 0xc00103ae60?})
        /go/pkg/mod/sigs.k8s.io/controller-runtime@v0.16.3/pkg/internal/controller/controller.go:316 +0x3cc
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem(0xc0006a4280, {0x24ef448, 0xc0005d72c0})
        /go/pkg/mod/sigs.k8s.io/controller-runtime@v0.16.3/pkg/internal/controller/controller.go:266 +0x1c9
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2()
        /go/pkg/mod/sigs.k8s.io/controller-runtime@v0.16.3/pkg/internal/controller/controller.go:227 +0x79
created by sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2 in goroutine 259
        /go/pkg/mod/sigs.k8s.io/controller-runtime@v0.16.3/pkg/internal/controller/controller.go:223 +0x565

This can be triggered as it follows

apiVersion: config.projectsveltos.io/v1alpha1
kind: Profile
metadata:
  name: cilium
  namespace: dtranchitella-1000-default
spec:
  clusterSelector: cni=cilium,  ## please notice the extra comma
  helmCharts:
  - chartName: cilium/cilium
    chartVersion: v1.14.5
    helmChartAction: Install
    releaseName: cilium
    releaseNamespace: kube-system
    repositoryName: cilium
    repositoryURL: https://helm.cilium.io/
  reloader: false
  stopMatchingBehavior: WithdrawPolicies
  syncMode: ContinuousWithDriftDetection

The error occurred on the last release (v0.21.0)

@gianlucam76 gianlucam76 self-assigned this Jan 6, 2024
@gianlucam76
Copy link
Member

Thank you @prometherion

gianlucam76 pushed a commit to gianlucam76/event-manager that referenced this issue Jan 11, 2024
gianlucam76 pushed a commit to gianlucam76/sveltosctl that referenced this issue Jan 11, 2024
gianlucam76 pushed a commit to gianlucam76/event-manager that referenced this issue Jan 11, 2024
gianlucam76 pushed a commit to gianlucam76/addon-constraint-controller that referenced this issue Jan 11, 2024
gianlucam76 pushed a commit to gianlucam76/access-manager that referenced this issue Jan 11, 2024
@gianlucam76 gianlucam76 added this to the v0.22.0 milestone Jan 11, 2024
AdamSadek added a commit to AdamSadek/sveltosctl that referenced this issue Jun 5, 2024
Forgot to check for clusterType

Fixing name of func and adding clusterType to function arguments

removing instance as it will now use the passed in dc

experimenting

Making adjustments

importing apierrors & metav1

fixing managed dc

directly setting ClusterName and ClusterType?

Resolving syntax errors

removing comma

Replacing registry name in 'manifest/sveltosctl.yaml' and 'Make File' script, from gianlucam76 to projectsveltos.

Techsupport

Introduce CRD and reconciler.

Following is an example of techsupport instance that every hour
collects from each cluster matching cluster selector env=fv:

1. logs of pods in namespace kube-system, collecting the last 10 minutes
worth of logs (600 seconds);
2. all namespaces will be dumped as YAML.

```
apiVersion: utils.projectsveltos.io/v1alpha1
kind: Techsupport
metadata:
  name: hourly
spec:
  clusterSelector: env=fv
  schedule: "00 * * * *"
  storage: /techsupport
  logs:
  - namespace: kube-system
    sinceSeconds: 600
  resources:
  - group: ""
    version: v1
    kind: Namespace
```

Prepare for release v0.6.0

Prepare dev to main sync

Prepare for release v0.7.0

Fix Makefile

Prepare dev to main merge

Prepare for release v0.7.1

Prepare dev to main merge

Release v0.7.2

Prepare dev to main merge

Advance pins

controller-tools: v0.11.3
controller-runtime: v0.14.5
cluster-api: v1.4.0-rc.1
k8s: v0.26.0

Advance pins

Collect EventSources/EventBasedAddOns

and referencedResources as part of snapshot

Collect HealthChecks as part of snapshot

Create CODE_OF_CONDUCT.md

Prepare for release v0.8.0

Prepare dev to main merge

Fix collection

Add CONTRIBUTING guidelines

Update CONTRIBUTING.md

Prepare dev to main merge

Fix for handling secrets

Rename show features to show addons

Kubernetes addons is the right terms to describe policies deployed by
sveltos in each managed cluster.

Advance pins

Advance pins

Fix README

Prepare for release v0.9.0

Prepare dev to main merge

Release v0.9.1

Prepare dev to main merge

Kustomize resources

Kustomize resources

Prepare for release v0.10.0

Prepare dev to main merge

Fix tag

Prepare for release v0.10.1

Prepare dev to main merge

Go version v1.20

Prepare for release v0.10.2

Prepare dev to main merge

Advance libsveltos pin

Prepare for release v0.11.0

Prepare dev to main merge

Rename AddonConstraint to AddonCompliance

Advance pins

- k8s v1.27.2
- controller-runtime v0.15.0
- cluster-api v1.5.0-beta.0

Prepare for release v0.12.0

Prepare dev to main merge

Add show resource command.

This command looks at all the HealthCheckReport instances and
display information about those.

For instance:

```
+-------------------------------------+--------------------------+----------------+-------------------------+----------------------------+
|               CLUSTER               |           GVK            |   NAMESPACE    |          NAME           |          MESSAGE           |
+-------------------------------------+--------------------------+----------------+-------------------------+----------------------------+
| default/sveltos-management-workload | apps/v1, Kind=Deployment | kube-system    | calico-kube-controllers | All replicas 1 are healthy |
|                                     |                          | kube-system    | coredns                 | All replicas 2 are healthy |
|                                     |                          | projectsveltos | sveltos-agent-manager   | All replicas 1 are healthy |
+-------------------------------------+--------------------------+----------------+-------------------------+----------------------------+
```

Command as also option --full.
When set full resource will be displayed

```
Cluster:  default/sveltos-management-workload
Object:  object:
  apiVersion: apps/v1
  kind: Deployment
  metadata:
    annotations:
      deployment.kubernetes.io/revision: "1"
    creationTimestamp: "2023-07-07T11:23:41Z"
    generation: 1
    labels:
      k8s-app: kube-dns
    managedFields:
    - apiVersion: apps/v1
      fieldsType: FieldsV1
      fieldsV1:
        f:metadata:
          f:labels:
            .: {}
            f:k8s-app: {}
        f:spec:
...
```

Fix log level for two messages

Prepare for release v0.13.0

Prepare dev to main merge

Fix README

Prepare for release v0.14.0

Prepare dev to main merge

Post release v0.14.0

Prepare for release v0.15.0

Prepare dev to main merge

Post release v0.15.0

Merge dev to main

Post release v0.15.1

Post release v0.15.2

Add bug template

Allowing apiGroup lib.projectsveltos.io to update

Fix CI

Add check to make sure manifest.yaml is in sync

Prepare release v0.15.3

Prepare dev to main merge

Post release v0.15.3

Fix tag

Add command to register management cluster

Prepare for release v0.16.0

Prepare dev to main merge

Post release v0.16.0

Prepare for release v0.17.0

Prepare dev to main merge

Post release v0.17.0

Advance libsveltos and clusterapi pins

Prepare for release v0.18.0

Prepare dev to main merge

Post release v0.18.0

Pick long running job fix

issue described [here](projectsveltos/libsveltos@33de363)

Prepare dev to main merge

Post release v0.18.1

Update go modules

Update tag

Prepare for release v0.19.0

Prepare dev to main merge

Post release v0.19.0

Prepare for release v0.20.0

Prepare dev to main merge

Post release v0.20.0

Add support for Profiles

Commands updated:
- show addons
- show dryrun
- roolback

Also snapshot collects Profile instances as well.

Update tag to dev

Golang v1.21

Move golangci-lint to v1.55.2
Temporarily disable depguard

Prepare for release v0.21.0

Fix sveltosctl rbac

Prepare dev to main merge

Post release v0.21.0

Return an error if clusterSelector is not parsable

[bug](projectsveltos/libsveltos#260)

Bump libsveltos

Prepare for release v0.22.0

Prepare dev to main merge

Post release v0.22.0

Prepare for release v0.23.0

Prepare dev to main merge

Post release v0.23.0

show addons: fix cluster filter

When displaying deployed add-ons, command allows filtering
by namespace and cluster name.
Because of an issue, filtering by cluster name was returning no result.

This PR fixes it.

```
sveltosctl show addons
+-----------------------------+---------------+-----------+----------------+---------+-------------------------------+------------------------+
|           CLUSTER           | RESOURCE TYPE | NAMESPACE |      NAME      | VERSION |             TIME              |        PROFILES        |
+-----------------------------+---------------+-----------+----------------+---------+-------------------------------+------------------------+
| default/clusterapi-workload | helm chart    | kyverno   | kyverno-latest | 3.0.1   | 2024-02-14 16:12:47 +0100 CET | ClusterProfile/kyverno |
| default/clusterapi-workload | helm chart    | nginx     | nginx-latest   | 0.14.0  | 2024-02-14 16:14:50 +0100 CET | ClusterProfile/kyverno |
+-----------------------------+---------------+-----------+----------------+---------+-------------------------------+------------------------+
```

Filter by namespace

```
sveltosctl show addons --namespace=default
+-----------------------------+---------------+-----------+----------------+---------+-------------------------------+------------------------+
|           CLUSTER           | RESOURCE TYPE | NAMESPACE |      NAME      | VERSION |             TIME              |        PROFILES        |
+-----------------------------+---------------+-----------+----------------+---------+-------------------------------+------------------------+
| default/clusterapi-workload | helm chart    | kyverno   | kyverno-latest | 3.0.1   | 2024-02-14 16:12:47 +0100 CET | ClusterProfile/kyverno |
| default/clusterapi-workload | helm chart    | nginx     | nginx-latest   | 0.14.0  | 2024-02-14 16:14:50 +0100 CET | ClusterProfile/kyverno |
+-----------------------------+---------------+-----------+----------------+---------+-------------------------------+------------------------+
```

Filter by namespace and cluster name

```
sveltosctl show addons --namespace=default  --cluster=clusterapi-workload
+-----------------------------+---------------+-----------+----------------+---------+-------------------------------+------------------------+
|           CLUSTER           | RESOURCE TYPE | NAMESPACE |      NAME      | VERSION |             TIME              |        PROFILES        |
+-----------------------------+---------------+-----------+----------------+---------+-------------------------------+------------------------+
| default/clusterapi-workload | helm chart    | kyverno   | kyverno-latest | 3.0.1   | 2024-02-14 16:12:47 +0100 CET | ClusterProfile/kyverno |
| default/clusterapi-workload | helm chart    | nginx     | nginx-latest   | 0.14.0  | 2024-02-14 16:14:50 +0100 CET | ClusterProfile/kyverno |
+-----------------------------+---------------+-----------+----------------+---------+-------------------------------+------------------------+
```

Prepare for release v0.24.0

Prepare for release v0.25.0

Prepare dev to main merge

Post release v0.25.0

Prepare for release v0.26.0

Prepare dev to main merge

Post release v0.26.0

Prepare for release v0.27.0

Prepare dev to main merge

Post release v0.27.0

Introduce new command to generate kubeconfig

```
sveltosctl generate kubeconfig --help
Usage:
  sveltosctl generate kubeconfig [options] [--namespace=<name>] [--serviceaccount=<name>] [--create] [--verbose]

     --namespace=<name>      The namespace of the ServiceAccount. If not specified, projectsveltos namespace will be used.
     --serviceaccount=<name> The name of the ServiceAccount. If not specified, projectsveltos will be used.
     --create                If a ServiceAccount with enough permissions is already present, do not set this flag.
                             Sveltos will generate a Kubeconfig associated to that ServiceAccount.
                             If a ServiceAccount with cluster admin permissions needs to be created, use this option.
                             When this option is set, this command will create necessary resources:
                             1. namespace if not existing already
                             2. serviceAccount if not existing already
                             3. ClusterRole with cluster admin permission
                             4. ClusterRoleBinding granting the serviceAccount cluster admin permissions
                             5. TokenRequest for the ServiceAccount

Options:
  -h --help                  Show this screen.
     --verbose               Verbose mode. Print each step.

Description:
  The generate kubeconfig command will generate a Kubeconfig that can later on be used to register the cluster.
```

Enhance generate kubeconfig description

This PR also modifies the `sveltosctl register cluster` command
by allowing labels to be specified.

For instance

```
sveltosctl register cluster \
  --namespace=monitoring \
  --cluster=prod-cluster \
  --kubeconfig=~/.kube/prod-cluster.config \
  --labels=environment=production,tier=backend
```

This command registers a cluster named "prod-cluster" in
the "monitoring" namespace with labels "environment=production" and "tier=backend".

Register a cluster means creates a SveltosCluster instance in the "monitoring"
namespace named "prod-cluster" with labels "environment=production" and "tier=backend".

Allow user to pass expiration

This option allows you to specify the desired validity period
(in seconds) for the token requested when generating a kubeconfig

Allow to register a cluster with one command

The sveltosctl register command is used to register a cluster.
It requires a Kubeconfig to be passed.

If user has a kubeconfig with multiple contexts, the option __fleet-cluster-context__
allows to specify the context for the cluster to be managed.

So with default context pointing to the management cluster, following command will:
1. create a ServiceAccount in the managed cluster (using cluster-1 context)
2. grant this ServiceAccount cluster-admin permission
3. create a TokenRequest for such account and a Kubeconfig with bearer token from the TokenRequest
4. create a SveltosCluster in the management cluster (so using default context) and a Secret
with kubeconfig generated in the step above

```
sveltosctl register cluster --namespace=gcp --cluster=cluster-1 --fleet-cluster-context=cluster-1 --labels=k1=v1,k2=v2
```

Fix issue with register cluster

When fleet-cluster-context is used, after switching context, get the new kubeconfig
and use it to get host and CA data.

Always reset context

When register cluster is used with the fleet-cluster-context
always reset context to default. In case there is any failure
this will avoid changing the context without resetting it back

Prepare for release v0.28.0

Prepare dev to main merge

Post release v0.28.0

Prepare for release v0.29.0

Fix lint

Prepare dev to main merge

Post release v0.29.0

extended log command to set severity for components in managed clusters

handle log-level command for managed clusters

argument mismatch fix, undefined instance fix, correct GetClientForManagedCluster to GetClientForCluster

error handling in set function, logical check in update operations.

new approach

not enough arguments fix attempt

dealing with some errors

adding new params to test

creating GetDebuggingConfiguration and adding dynamic switching of clusterName

reverting changes in client.go as I misunderstood a part and maing change in debugging config file also removed default isntance

update params in test files

I thought there was a typo

unset missing the new params!

lint erros

new approach in utils.go

param order fix

reducing errors

removing extra params

removing nil return from collectlogconfig

trying a new fix

i dont think theres a point in returning the namespace and clustername in collectloglevelconfig

tried a new approach. should be better

making change sin debuggingconfig.go file

making the changes in the debbuging config test file

should fix the failing tests

mispell is on purpose, i keep forgetting!

fixing lint errors to make test vars constants

forgot to add change in utils.go debug config section

reverting change

adding clusterType and making additional changes

clusterproxy.GetSveltosKubernetesClient to get the client pointing to the managed cluster

might be an issue with k8sAccess redeclaration

Misunderstanding. This should be better

adding cluster type to req object

resolving some errors

adding logger as param

getting logger in a different way

logger is being tedious. debugging

fixing logger

new approach to get logger

getting logger new approach

new approach for logger

Passing it as a parameter

using correct GetKubernetesClient

getting logger

adding  defaultInstanceName back

attempting to resolve error in test for debuggingconfig

attempt to fix test, different approach

reverting change

adding values instead of empty

testing fix

testing dc name

no need for addition in set_test

adding cluster type and namespace to DC

adding type and namespace to DC

Testing new approach for debuggingconfiguratgions

removing Type

new fixes

reveritng back to previous change.

reverting back to previous version

Changing expect test from Namespace to Name

Reading optional arguments correctly.

Syntax fix

Reading optional arguments correctly for show.go

Prepare for release v0.30.0

Prepare dev to main merge

Post release v0.30.0

Prepare for release v0.31.0

Prepare dev to main merge

Post release v0.31.0
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

No branches or pull requests

2 participants