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.

Sign up for GitHub

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-1737]: Check if CSM is Authorization Proxy Server by checking module name #893

Merged
merged 6 commits into from
Feb 14, 2025
Merged

[bug-1737]: Check if CSM is Authorization Proxy Server by checking module name #893

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b0d1fc3
use module name not cr name
atye Feb 6, 2025
c13199e
bump coverage
atye Feb 13, 2025
957e17f
Merge branch 'main' into usr/atye/bug-1737-authorization-name
atye Feb 13, 2025
d3dd4c0
Merge branch 'main' into usr/atye/bug-1737-authorization-name
donatwork Feb 13, 2025
3d074f9
Merge branch 'main' into usr/atye/bug-1737-authorization-name
donatwork Feb 13, 2025
7c60f2f
go mod tidy in tests/e2e
atye Feb 14, 2025
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
19 changes: 17 additions & 2 deletions pkg/utils/status.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func getDeploymentStatus(ctx context.Context, instance *csmv1.ContainerStorageMo
log.Infof("getting deployment status for cluster: %s", cluster.ClusterID)
msg += fmt.Sprintf("error message for %s \n", cluster.ClusterID)

if instance.GetName() == "" || instance.GetName() == string(csmv1.Authorization) || instance.GetName() == string(csmv1.ApplicationMobility) {
if instance.GetName() == "" || isAuthorizationProxyServer(instance) || instance.GetName() == string(csmv1.ApplicationMobility) {
log.Infof("Not a driver instance, will not check deploymentstatus")
return emptyStatus, nil
}
Expand Down Expand Up @@ -218,7 +218,7 @@ func calculateState(ctx context.Context, instance *csmv1.ContainerStorageModule,
// Auth proxy has no daemonset. Putting this if/else in here and setting nodeStatusGood to true by
// default is a little hacky but will be fixed when we refactor the status code in CSM 1.10 or 1.11
log.Infof("instance.GetName() is %s", instance.GetName())
if instance.GetName() != "" && instance.GetName() != string(csmv1.Authorization) && instance.GetName() != string(csmv1.ApplicationMobility) {
if instance.GetName() != "" && !isAuthorizationProxyServer(instance) && instance.GetName() != string(csmv1.ApplicationMobility) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain this change? Does the authorization proxy server only exist for auth v1?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

instance.GetName() != string(csmv1.Authorization) checks if the Authorization CSM name is authorization. Assuming the user doesn't modify the default name in the sample, it'll work. If the user changes the CSM name, this check fails and reconciling will have issues.

A better check to determine if a CSM is the Authorization Server is to check the module name for authoriztion-proxy-server, which isAuthorizationProxyServer does.

expected, nodeStatus, daemonSetErr := getDaemonSetStatus(ctx, instance, r)
newStatus.NodeStatus = nodeStatus
if daemonSetErr != nil {
Expand Down Expand Up @@ -768,10 +768,25 @@ func authProxyStatusCheck(ctx context.Context, instance *csmv1.ContainerStorageM
log.Infof("%s component not running in auth proxy deployment", deployment.Name)
return false, nil
}
case "authorization-controller":
if !checkFn(&deployment) {
log.Infof("%s component not running in auth proxy deployment", deployment.Name)
return false, nil
}
}

}

log.Info("auth proxy deployment successful")

return true, nil
}

func isAuthorizationProxyServer(cr *csmv1.ContainerStorageModule) bool {
for _, m := range cr.Spec.Modules {
if m.Name == csmv1.AuthorizationServer {
return true
}
}
return false
}
54 changes: 52 additions & 2 deletions pkg/utils/status_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,24 @@ func TestGetDeploymentStatus(t *testing.T) {
wantErr: false,
},
{
name: "Test getDeploymentStatus when instance name is authorization",
name: "Test getDeploymentStatus when instance is authorization proxy server",
args: args{
ctx: context.Background(),
instance: createCSM(string(csmv1.Authorization), "", csmv1.PowerFlex, csmv1.Replication, true, nil),
instance: createCSM("authorization", "", "", csmv1.AuthorizationServer, true, nil),
r: &fakeReconcile,
},
want: csmv1.PodStatus{
Available: "0",
Desired: "0",
Failed: "0",
},
wantErr: false,
},
{
name: "Test getDeploymentStatus when instance is authorization proxy server with non-default name",
args: args{
ctx: context.Background(),
instance: createCSM("csm-authorization", "", "", csmv1.AuthorizationServer, true, nil),
r: &fakeReconcile,
},
want: csmv1.PodStatus{
Expand Down Expand Up @@ -1499,6 +1513,19 @@ func TestAuthProxyStatusCheck(t *testing.T) {
Replicas: &i32One,
},
}
deployment11 := appsv1.Deployment{
ObjectMeta: metav1.ObjectMeta{
Name: "authorization-controller",
Namespace: "test-namespace",
},
Status: appsv1.DeploymentStatus{
ReadyReplicas: 1,
},
Spec: appsv1.DeploymentSpec{
Replicas: &i32One,
},
}

err = ctrlClient.Create(ctx, &deployment1)
assert.NoError(t, err, "failed to create client object during test setup")
err = ctrlClient.Create(ctx, &deployment2)
Expand All @@ -1519,6 +1546,8 @@ func TestAuthProxyStatusCheck(t *testing.T) {
assert.NoError(t, err, "failed to create client object during test setup")
err = ctrlClient.Create(ctx, &deployment10)
assert.NoError(t, err, "failed to create client object during test setup")
err = ctrlClient.Create(ctx, &deployment11)
assert.NoError(t, err, "failed to create client object during test setup")

// Create a fake instance of ReconcileCSM
fakeReconcile := FakeReconcileCSM{
Expand Down Expand Up @@ -1698,6 +1727,19 @@ func TestAuthProxyStatusCheckError(t *testing.T) {
},
}

authorizationControllerDeployment := appsv1.Deployment{
ObjectMeta: metav1.ObjectMeta{
Name: "authorization-controller",
Namespace: "test-namespace",
},
Status: appsv1.DeploymentStatus{
ReadyReplicas: 0,
},
Spec: appsv1.DeploymentSpec{
Replicas: &i32One,
},
}

err = ctrlClient.Create(ctx, &nginxDeployment)
assert.NoError(t, err, "failed to create client object during test setup")

Expand Down Expand Up @@ -1784,6 +1826,14 @@ func TestAuthProxyStatusCheckError(t *testing.T) {

recreateDeployment(ctx, t, ctrlClient, &tenantServiceDeployment, 1)

err = ctrlClient.Create(ctx, &authorizationControllerDeployment)
assert.NoError(t, err, "failed to create client object during test setup")

_, err = authProxyStatusCheck(ctx, &csm, &fakeReconcile, nil)
assert.Nil(t, err)

recreateDeployment(ctx, t, ctrlClient, &authorizationControllerDeployment, 1)

deleteDeployments(ctx, t, ctrlClient, &nginxDeployment, &certManagerDeployment, &certManagerCainjectorDeployment, &certManagerWebhookDeployment, &proxyServerDeployment, &redisCommanderDeployment, &redisPrimaryDeployment, &roleServiceDeployment, &storageServiceDeployment, &tenantServiceDeployment)
}

Expand Down
65 changes: 33 additions & 32 deletions tests/e2e/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,27 +8,27 @@ require (
github.com/dell/csm-operator v0.0.0
github.com/onsi/ginkgo/v2 v2.22.2
github.com/onsi/gomega v1.36.2
golang.org/x/mod v0.22.0
golang.org/x/mod v0.23.0
k8s.io/api v0.32.0
k8s.io/apimachinery v0.32.0
k8s.io/client-go v0.32.0
k8s.io/kubernetes v1.32.2
k8s.io/utils v0.0.0-20241104100929-3ea5e8cea738
sigs.k8s.io/controller-runtime v0.19.4
k8s.io/utils v0.0.0-20241210054802-24370beab758
sigs.k8s.io/controller-runtime v0.20.1
sigs.k8s.io/yaml v1.4.0
)

require (
cel.dev/expr v0.18.0 // indirect
cel.dev/expr v0.19.1 // indirect
github.com/JeffAshton/win_pdh v0.0.0-20161109143554-76bb4ee9f0ab // indirect
github.com/Microsoft/go-winio v0.6.2 // indirect
github.com/NYTimes/gziphandler v1.1.1 // indirect
github.com/antlr4-go/antlr/v4 v4.13.0 // indirect
github.com/antlr4-go/antlr/v4 v4.13.1 // indirect
github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2 // indirect
github.com/beorn7/perks v1.0.1 // indirect
github.com/blang/semver/v4 v4.0.0 // indirect
github.com/cenkalti/backoff/v4 v4.3.0 // indirect
github.com/cert-manager/cert-manager v1.16.2 // indirect
github.com/cert-manager/cert-manager v1.17.1 // indirect
github.com/cespare/xxhash/v2 v2.3.0 // indirect
github.com/containerd/containerd/api v1.7.19 // indirect
github.com/containerd/errdefs v0.1.0 // indirect
Expand All @@ -55,22 +55,22 @@ require (
github.com/godbus/dbus/v5 v5.1.0 // indirect
github.com/gogo/protobuf v1.3.2 // indirect
github.com/golang/protobuf v1.5.4 // indirect
github.com/google/btree v1.0.1 // indirect
github.com/google/btree v1.1.3 // indirect
github.com/google/cadvisor v0.51.0 // indirect
github.com/google/cel-go v0.22.0 // indirect
github.com/google/cel-go v0.22.1 // indirect
github.com/google/gnostic-models v0.6.9 // indirect
github.com/google/go-cmp v0.6.0 // indirect
github.com/google/gofuzz v1.2.0 // indirect
github.com/google/pprof v0.0.0-20241210010833-40e02aabc2ad // indirect
github.com/google/uuid v1.6.0 // indirect
github.com/gorilla/websocket v1.5.1 // indirect
github.com/gorilla/websocket v1.5.3 // indirect
github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0 // indirect
github.com/grpc-ecosystem/grpc-gateway/v2 v2.20.0 // indirect
github.com/grpc-ecosystem/grpc-gateway/v2 v2.25.1 // indirect
github.com/inconshreveable/mousetrap v1.1.0 // indirect
github.com/josharian/intern v1.0.0 // indirect
github.com/json-iterator/go v1.1.12 // indirect
github.com/karrick/godirwalk v1.17.0 // indirect
github.com/klauspost/compress v1.17.9 // indirect
github.com/klauspost/compress v1.17.11 // indirect
github.com/kylelemons/godebug v1.1.0 // indirect
github.com/mailru/easyjson v0.9.0 // indirect
github.com/mistifyio/go-zfs v2.1.2-0.20190413222219-f784269be439+incompatible // indirect
Expand All @@ -86,7 +86,7 @@ require (
github.com/opencontainers/runtime-spec v1.2.0 // indirect
github.com/opencontainers/selinux v1.11.1 // indirect
github.com/pkg/errors v0.9.1 // indirect
github.com/prometheus/client_golang v1.20.4 // indirect
github.com/prometheus/client_golang v1.20.5 // indirect
github.com/prometheus/client_model v0.6.1 // indirect
github.com/prometheus/common v0.61.0 // indirect
github.com/prometheus/procfs v0.15.1 // indirect
Expand All @@ -96,33 +96,34 @@ require (
github.com/stoewer/go-strcase v1.3.0 // indirect
github.com/vmware-tanzu/velero v1.15.1 // indirect
github.com/x448/float16 v0.8.4 // indirect
go.etcd.io/etcd/api/v3 v3.5.16 // indirect
go.etcd.io/etcd/client/pkg/v3 v3.5.16 // indirect
go.etcd.io/etcd/client/v3 v3.5.16 // indirect
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.54.0 // indirect
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.54.0 // indirect
go.opentelemetry.io/otel v1.29.0 // indirect
go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.28.0 // indirect
go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.27.0 // indirect
go.opentelemetry.io/otel/metric v1.29.0 // indirect
go.opentelemetry.io/otel/sdk v1.28.0 // indirect
go.opentelemetry.io/otel/trace v1.29.0 // indirect
go.opentelemetry.io/proto/otlp v1.3.1 // indirect
go.etcd.io/etcd/api/v3 v3.5.17 // indirect
go.etcd.io/etcd/client/pkg/v3 v3.5.17 // indirect
go.etcd.io/etcd/client/v3 v3.5.17 // indirect
go.opentelemetry.io/auto/sdk v1.1.0 // indirect
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.58.0 // indirect
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.58.0 // indirect
go.opentelemetry.io/otel v1.33.0 // indirect
go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.33.0 // indirect
go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.33.0 // indirect
go.opentelemetry.io/otel/metric v1.33.0 // indirect
go.opentelemetry.io/otel/sdk v1.33.0 // indirect
go.opentelemetry.io/otel/trace v1.33.0 // indirect
go.opentelemetry.io/proto/otlp v1.4.0 // indirect
go.uber.org/multierr v1.11.0 // indirect
go.uber.org/zap v1.27.0 // indirect
golang.org/x/crypto v0.32.0 // indirect
golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 // indirect
golang.org/x/exp v0.0.0-20241217172543-b2144cdd0a67 // indirect
golang.org/x/net v0.34.0 // indirect
golang.org/x/oauth2 v0.24.0 // indirect
golang.org/x/sync v0.10.0 // indirect
golang.org/x/sys v0.29.0 // indirect
golang.org/x/term v0.28.0 // indirect
golang.org/x/text v0.21.0 // indirect
golang.org/x/time v0.7.0 // indirect
golang.org/x/time v0.8.0 // indirect
golang.org/x/tools v0.28.0 // indirect
google.golang.org/genproto/googleapis/api v0.0.0-20240827150818-7e3bb234dfed // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20240903143218-8af14fe29dc1 // indirect
google.golang.org/grpc v1.66.2 // indirect
google.golang.org/genproto/googleapis/api v0.0.0-20241219192143-6b3ec007d9bb // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20241219192143-6b3ec007d9bb // indirect
google.golang.org/grpc v1.69.2 // indirect
google.golang.org/protobuf v1.36.1 // indirect
gopkg.in/evanphx/json-patch.v4 v4.12.0 // indirect
gopkg.in/inf.v0 v0.9.1 // indirect
Expand All @@ -140,15 +141,15 @@ require (
k8s.io/dynamic-resource-allocation v0.0.0 // indirect
k8s.io/klog/v2 v2.130.1 // indirect
k8s.io/kms v0.32.0 // indirect
k8s.io/kube-openapi v0.0.0-20241105132330-32ad38e42d3f // indirect
k8s.io/kube-openapi v0.0.0-20241212222426-2c72e554b1e7 // indirect
k8s.io/kube-scheduler v0.0.0 // indirect
k8s.io/kubectl v0.32.0 // indirect
k8s.io/kubelet v0.32.0 // indirect
k8s.io/mount-utils v0.0.0 // indirect
k8s.io/pod-security-admission v0.32.0 // indirect
sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.31.0 // indirect
sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.31.1 // indirect
sigs.k8s.io/gateway-api v1.2.1 // indirect
sigs.k8s.io/json v0.0.0-20241010143419-9aa6b5e7a4b3 // indirect
sigs.k8s.io/json v0.0.0-20241014173422-cfa47c3a1cc8 // indirect
sigs.k8s.io/structured-merge-diff/v4 v4.5.0 // indirect
)

Expand Down
Loading
Loading