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

Added a new annotation: vertica.com/superuser-name #578

Merged
merged 6 commits into from
Nov 8, 2023
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
13 changes: 13 additions & 0 deletions .github/workflows/e2e-leg-4.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ on:
vertica-deployment-method:
type: string
required: false
vertica-superuser-name:
type: string
required: false
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we set the default?

Suggested change
required: false
required: false
default: "myadmin"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I set the default value in workflow_dispatch.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think it's picking that up though. Workflow dispatch is when you call the workflow directly from the REST API. Workflow call is when it is called by another workflow, which is what we are doing.

default: myadmin
secrets:
DOCKERHUB_USERNAME:
description: 'When working with images from docker.io, this is the username for login purposes'
Expand Down Expand Up @@ -44,6 +48,11 @@ on:
options:
- admintools
- vclusterops
vertica-superuser-name:
description: 'Vertica superuser name'
type: string
required: false
default: myadmin

jobs:

Expand Down Expand Up @@ -72,6 +81,10 @@ jobs:
export E2E_TEST_DIRS="tests/e2e-leg-4"
export VERTICA_DEPLOYMENT_METHOD=${{ inputs.vertica-deployment-method }}
if [ "${VERTICA_DEPLOYMENT_METHOD}" != "vclusterops" ]; then E2E_TEST_DIRS+=" tests/e2e-leg-4-at-only"; fi
export VERTICA_SUPERUSER_NAME=${{ inputs.vertica-superuser-name }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think we are setting this correctly. When I look at your CI run I see dbadmin being used:

Vertica server image name: docker.io/vertica/vertica-k8s-private:latest-minimal-master
Base vertica server image name for upgrade tests: <not-set>
Vertica logger image name: vertica-logger:kind
Endpoint: https://minio.kuttl-e2e-s3/
Protocol: s3://
Communal Path Prefix: s3://nimbusdb/
Using private registry: NO
Add server mounts: NO
Deployment method: vclusterops
Vertica superuser name: dbadmin

https://github.com/vertica/vertica-kubernetes/actions/runs/6785933635/job/18448129113

The new parameter was added to workflow_dispatch. That gets called when you trigger just this workflow. There are a set of parameters for workflow_call. This is used when another workflow calls this workflow, which is what happens during a PR. Can you add the new parameter to workflow_call?

if [ -z "${VERTICA_SUPERUSER_NAME}" -o "${VERTICA_DEPLOYMENT_METHOD}" != "vclusterops" ]; then
export VERTICA_SUPERUSER_NAME="dbadmin"
fi
# All helm deployments will use cert-manager to create the webhook cert
export HELM_OVERRIDES="--set webhook.certSource=cert-manager"
scripts/run-k8s-int-tests.sh -s -e tests/external-images-s3-ci.txt
Expand Down
4 changes: 2 additions & 2 deletions api/v1/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,7 @@ func (s *SubclusterStatus) InstallCount() int32 {
return c
}

// GetVerticaUser returns the user name associated with the passwordSecret.
// GetVerticaUser returns the name of Vertica superuser generated in database creation.
func (v *VerticaDB) GetVerticaUser() string {
return "dbadmin"
return vmeta.GetSuperuserName(v.Annotations)
}
3 changes: 0 additions & 3 deletions api/v1beta1/verticadb_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -449,9 +449,6 @@ const (
AutoUpgrade UpgradePolicyType = "Auto"
)

// SuperUser is an automatically-created user in database creation
const SuperUser = "dbadmin"

type HTTPServerModeType string

const (
Expand Down
5 changes: 5 additions & 0 deletions changes/unreleased/Added-20231107-093813.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
kind: Added
body: Ability to control the name of the superuser
time: 2023-11-07T09:38:13.490515872-04:00
custom:
Issue: "578"
14 changes: 7 additions & 7 deletions pkg/cmds/cmds_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,25 +36,25 @@ var _ = Describe("k8s/cmds", func() {

It("should add the password option to vsql command", func() {
cmd := []string{"-tAc", "select 1"}
fpr := &FakePodRunner{SUPassword: "vertica"}
fpr := &FakePodRunner{VerticaSUPassword: "vertica"}
podName := types.NamespacedName{Namespace: "default", Name: "vdb-pod"}
_, _, _ = fpr.ExecVSQL(ctx, podName, "server", cmd...)
lastCall := fpr.FindCommands("vsql", "--password", fpr.SUPassword, "-tAc", "select 1")
lastCall := fpr.FindCommands("vsql", "--password", fpr.VerticaSUPassword, "-tAc", "select 1")
Expect(len(lastCall)).Should(Equal(1))
})

It("should add password option for db_add_node", func() {
cmd := []string{"-t", "db_add_node"}
fpr := &FakePodRunner{SUPassword: "vertica"}
fpr := &FakePodRunner{VerticaSUPassword: "vertica"}
podName := types.NamespacedName{Namespace: "default", Name: "vdb-pod"}
_, _, _ = fpr.ExecAdmintools(ctx, podName, "server", cmd...)
lastCall := fpr.FindCommands("/opt/vertica/bin/admintools", "-t", "db_add_node", "--password", fpr.SUPassword)
lastCall := fpr.FindCommands("/opt/vertica/bin/admintools", "-t", "db_add_node", "--password", fpr.VerticaSUPassword)
Expect(len(lastCall)).Should(Equal(1))
})

It("should not add password to an admintools' tool which does not support it", func() {
cmd := []string{"-t", "list_allnodes"}
fpr := &FakePodRunner{SUPassword: "vertica"}
fpr := &FakePodRunner{VerticaSUPassword: "vertica"}
podName := types.NamespacedName{Namespace: "default", Name: "vdb-pod"}
_, _, _ = fpr.ExecAdmintools(ctx, podName, "server", cmd...)
lastCall := fpr.FindCommands("/opt/vertica/bin/admintools", "-t", "list_allnodes")
Expand All @@ -63,7 +63,7 @@ var _ = Describe("k8s/cmds", func() {

It("should not add password to vsql command", func() {
cmd := []string{"-tAc", "select 1"}
fpr := &FakePodRunner{SUPassword: ""}
fpr := &FakePodRunner{VerticaSUPassword: ""}
podName := types.NamespacedName{Namespace: "default", Name: "vdb-pod"}
_, _, _ = fpr.ExecVSQL(ctx, podName, "server", cmd...)
lastCall := fpr.FindCommands("vsql", "-tAc", "select 1")
Expand All @@ -72,7 +72,7 @@ var _ = Describe("k8s/cmds", func() {

It("should not add password to admintools", func() {
cmd := []string{"-t", "db_add_node"}
fpr := &FakePodRunner{SUPassword: ""}
fpr := &FakePodRunner{VerticaSUPassword: ""}
podName := types.NamespacedName{Namespace: "default", Name: "vdb-pod"}
_, _, _ = fpr.ExecAdmintools(ctx, podName, "server", cmd...)
lastCall := fpr.FindCommands("/opt/vertica/bin/admintools", "-t", "db_add_node")
Expand Down
22 changes: 13 additions & 9 deletions pkg/cmds/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,15 @@ type PodRunner interface {
}

type ClusterPodRunner struct {
Log logr.Logger
Cfg *rest.Config
SUPassword string
Log logr.Logger
Cfg *rest.Config
VerticaSUName string // vertica superuser generated in database creation
VerticaSUPassword string
}

// MakeClusterPodRunnerr will build a ClusterPodRunner object
func MakeClusterPodRunner(log logr.Logger, cfg *rest.Config, passwd string) *ClusterPodRunner {
return &ClusterPodRunner{Log: log, Cfg: cfg, SUPassword: passwd}
func MakeClusterPodRunner(log logr.Logger, cfg *rest.Config, verticaSUName, passwd string) *ClusterPodRunner {
return &ClusterPodRunner{Log: log, Cfg: cfg, VerticaSUName: verticaSUName, VerticaSUPassword: passwd}
}

// logInfoCmd calls log function for the given command
Expand Down Expand Up @@ -140,14 +141,14 @@ func (c *ClusterPodRunner) CopyToPod(ctx context.Context, podName types.Namespac
// ExecVSQL appends options to the vsql command and calls ExecInPod
func (c *ClusterPodRunner) ExecVSQL(ctx context.Context, podName types.NamespacedName,
contName string, command ...string) (stdout, stderr string, err error) {
command = UpdateVsqlCmd(c.SUPassword, command...)
command = UpdateVsqlCmd(c.VerticaSUName, c.VerticaSUPassword, command...)
return c.ExecInPod(ctx, podName, contName, command...)
}

// ExecAdmintools appends options to the admintools command and calls ExecInPod
func (c *ClusterPodRunner) ExecAdmintools(ctx context.Context, podName types.NamespacedName,
contName string, command ...string) (stdout, stderr string, err error) {
command = UpdateAdmintoolsCmd(c.SUPassword, command...)
command = UpdateAdmintoolsCmd(c.VerticaSUPassword, command...)
return c.ExecInPod(ctx, podName, contName, command...)
}

Expand All @@ -164,10 +165,13 @@ func (c *ClusterPodRunner) DumpAdmintoolsConf(ctx context.Context, podName types
}

// UpdateVsqlCmd generates a vsql command appending the options we need
func UpdateVsqlCmd(passwd string, cmd ...string) []string {
func UpdateVsqlCmd(suName, passwd string, cmd ...string) []string {
prefix := []string{"vsql"}
if suName != "" {
prefix = append(prefix, "-U", suName)
}
if passwd != "" {
prefix = []string{"vsql", "--password", passwd}
prefix = append(prefix, "--password", passwd)
}
cmd = append(prefix, cmd...)
return cmd
Expand Down
8 changes: 5 additions & 3 deletions pkg/cmds/fake.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,10 @@ type FakePodRunner struct {
// The commands that were issue. The commands are in the same order that
// commands were received. This is filled in by ExecInPod and can be inspected.
Histories []CmdHistory
// fake username
VerticaSUName string
// fake password
SUPassword string
VerticaSUPassword string
}

// CmdResults stores the command result. The key is the pod name.
Expand Down Expand Up @@ -76,14 +78,14 @@ func (f *FakePodRunner) ExecInPod(_ context.Context, podName types.NamespacedNam
// ExecAdmintools calls ExecInPod
func (f *FakePodRunner) ExecAdmintools(ctx context.Context, podName types.NamespacedName,
contName string, command ...string) (stdout, stderr string, err error) {
command = UpdateAdmintoolsCmd(f.SUPassword, command...)
command = UpdateAdmintoolsCmd(f.VerticaSUPassword, command...)
return f.ExecInPod(ctx, podName, contName, command...)
}

// ExecVSQL calls ExecInPod
func (f *FakePodRunner) ExecVSQL(ctx context.Context, podName types.NamespacedName,
contName string, command ...string) (stdout, stderr string, err error) {
command = UpdateVsqlCmd(f.SUPassword, command...)
command = UpdateVsqlCmd(f.VerticaSUName, f.VerticaSUPassword, command...)
return f.ExecInPod(ctx, podName, contName, command...)
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/controllers/vdb/verticadb_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func (r *VerticaDBReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
if err != nil {
return ctrl.Result{}, err
}
prunner := cmds.MakeClusterPodRunner(log, r.Cfg, passwd)
prunner := cmds.MakeClusterPodRunner(log, r.Cfg, vdb.GetVerticaUser(), passwd)
// We use the same pod facts for all reconcilers. This allows to reuse as
// much as we can. Some reconcilers will purposely invalidate the facts if
// it is known they did something to make them stale.
Expand Down
17 changes: 17 additions & 0 deletions pkg/meta/annotations.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,14 @@ const (
BuildRefAnnotation = "vertica.com/buildRef"
// Annotation for the database's revive_instance_id
ReviveInstanceIDAnnotation = "vertica.com/revive-instance-id"

// Annotation for a customized superuser name. This annotation can be used
// when vclusterops annotation is set to true. It can explicitly specify the
// name of vertica superuser that is generated in database creation. If this
// annotation is not provided or vclusterops annotation is set to false, the
// default value "dbadmin" will be used.
SuperuserNameAnnotation = "vertica.com/superuser-name"
SuperuserNameDefaultValue = "dbadmin"
)

// IsPauseAnnotationSet will check the annotations for a special value that will
Expand Down Expand Up @@ -192,6 +200,15 @@ func IncludeUIDInPath(annotations map[string]string) bool {
return lookupBoolAnnotation(annotations, IncludeUIDInPathAnnotation, false /* default value */)
}

// GetSuperuserName returns the name of customized vertica superuser name
// for vclusterops style of deployments.
func GetSuperuserName(annotations map[string]string) string {
if UseVClusterOps(annotations) {
return lookupStringAnnotation(annotations, SuperuserNameAnnotation, SuperuserNameDefaultValue)
}
return SuperuserNameDefaultValue
}

// lookupBoolAnnotation is a helper function to lookup a specific annotation and
// treat it as if it were a boolean.
func lookupBoolAnnotation(annotations map[string]string, annotation string, defaultValue bool) bool {
Expand Down
3 changes: 1 addition & 2 deletions pkg/vadmin/add_node_vc.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (

vops "github.com/vertica/vcluster/vclusterops"
"github.com/vertica/vcluster/vclusterops/vstruct"
vapi "github.com/vertica/vertica-kubernetes/api/v1"
"github.com/vertica/vertica-kubernetes/pkg/events"
"github.com/vertica/vertica-kubernetes/pkg/net"
"github.com/vertica/vertica-kubernetes/pkg/vadmin/opts/addnode"
Expand Down Expand Up @@ -79,7 +78,7 @@ func (v *VClusterOps) genAddNodeOptions(s *addnode.Parms, certs *HTTPSCerts) vop
opts.Key = certs.Key
opts.Cert = certs.Cert
opts.CaCert = certs.CaCert
*opts.UserName = vapi.SuperUser
*opts.UserName = v.VDB.GetVerticaUser()
opts.Password = &v.Password

return opts
Expand Down
3 changes: 1 addition & 2 deletions pkg/vadmin/add_sc_vc.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (

vops "github.com/vertica/vcluster/vclusterops"
"github.com/vertica/vcluster/vclusterops/vstruct"
vapi "github.com/vertica/vertica-kubernetes/api/v1"
"github.com/vertica/vertica-kubernetes/pkg/net"
"github.com/vertica/vertica-kubernetes/pkg/vadmin/opts/addsc"
)
Expand Down Expand Up @@ -60,7 +59,7 @@ func (v *VClusterOps) genAddSubclusterOptions(s *addsc.Parms) vops.VAddSubcluste
opts.IsPrimary = &s.IsPrimary

// auth options
*opts.UserName = vapi.SuperUser
*opts.UserName = v.VDB.GetVerticaUser()
opts.Password = &v.Password
*opts.HonorUserInput = true

Expand Down
3 changes: 1 addition & 2 deletions pkg/vadmin/create_db_vc.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (

vops "github.com/vertica/vcluster/vclusterops"
"github.com/vertica/vcluster/vclusterops/vstruct"
vapi "github.com/vertica/vertica-kubernetes/api/v1"
"github.com/vertica/vertica-kubernetes/pkg/events"
"github.com/vertica/vertica-kubernetes/pkg/net"
"github.com/vertica/vertica-kubernetes/pkg/vadmin/opts/createdb"
Expand Down Expand Up @@ -87,7 +86,7 @@ func (v *VClusterOps) genCreateDBOptions(s *createdb.Parms, certs *HTTPSCerts) v
opts.Key = certs.Key
opts.Cert = certs.Cert
opts.CaCert = certs.CaCert
*opts.UserName = vapi.SuperUser
*opts.UserName = v.VDB.GetVerticaUser()
if v.Password != "" {
opts.Password = &v.Password
}
Expand Down
3 changes: 1 addition & 2 deletions pkg/vadmin/fetch_node_state_vc.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (

vops "github.com/vertica/vcluster/vclusterops"
"github.com/vertica/vcluster/vclusterops/vstruct"
vapi "github.com/vertica/vertica-kubernetes/api/v1"
"github.com/vertica/vertica-kubernetes/pkg/net"
"github.com/vertica/vertica-kubernetes/pkg/vadmin/opts/fetchnodestate"
ctrl "sigs.k8s.io/controller-runtime"
Expand Down Expand Up @@ -68,7 +67,7 @@ func (v *VClusterOps) genFetchNodeStateOptions(s *fetchnodestate.Parms) vops.VFe
opts.Ipv6 = vstruct.MakeNullableBool(net.IsIPv6(s.InitiatorIP))

// auth options
*opts.UserName = vapi.SuperUser
*opts.UserName = v.VDB.GetVerticaUser()
opts.Password = &v.Password
*opts.HonorUserInput = true

Expand Down
2 changes: 1 addition & 1 deletion pkg/vadmin/re_ip_vc.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ func (v *VClusterOps) genReIPOptions(s *reip.Parms, certs *HTTPSCerts) vops.VReI
opts.Key = certs.Key
opts.Cert = certs.Cert
opts.CaCert = certs.CaCert
*opts.UserName = vapi.SuperUser
*opts.UserName = v.VDB.GetVerticaUser()
opts.Password = &v.Password
*opts.HonorUserInput = true

Expand Down
3 changes: 1 addition & 2 deletions pkg/vadmin/remove_node_vc.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (

vops "github.com/vertica/vcluster/vclusterops"
"github.com/vertica/vcluster/vclusterops/vstruct"
vapi "github.com/vertica/vertica-kubernetes/api/v1"
"github.com/vertica/vertica-kubernetes/pkg/net"
"github.com/vertica/vertica-kubernetes/pkg/vadmin/opts/removenode"
)
Expand Down Expand Up @@ -66,7 +65,7 @@ func (v *VClusterOps) genRemoveNodeOptions(s *removenode.Parms, certs *HTTPSCert
opts.Key = certs.Key
opts.Cert = certs.Cert
opts.CaCert = certs.CaCert
*opts.UserName = vapi.SuperUser
*opts.UserName = v.VDB.GetVerticaUser()
opts.Password = &v.Password

return opts
Expand Down
3 changes: 1 addition & 2 deletions pkg/vadmin/remove_sc_vc.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"github.com/vertica/vcluster/rfc7807"
vops "github.com/vertica/vcluster/vclusterops"
"github.com/vertica/vcluster/vclusterops/vstruct"
vapi "github.com/vertica/vertica-kubernetes/api/v1"
"github.com/vertica/vertica-kubernetes/pkg/net"
"github.com/vertica/vertica-kubernetes/pkg/vadmin/opts/removesc"
)
Expand Down Expand Up @@ -81,7 +80,7 @@ func (v *VClusterOps) genRemoveSubclusterOptions(s *removesc.Parms, certs *HTTPS
opts.Key = certs.Key
opts.Cert = certs.Cert
opts.CaCert = certs.CaCert
*opts.UserName = vapi.SuperUser
*opts.UserName = v.VDB.GetVerticaUser()
opts.Password = &v.Password

return opts
Expand Down
3 changes: 1 addition & 2 deletions pkg/vadmin/restart_node_vc.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (

vops "github.com/vertica/vcluster/vclusterops"
"github.com/vertica/vcluster/vclusterops/vstruct"
vapi "github.com/vertica/vertica-kubernetes/api/v1"
"github.com/vertica/vertica-kubernetes/pkg/net"
"github.com/vertica/vertica-kubernetes/pkg/vadmin/opts/restartnode"
ctrl "sigs.k8s.io/controller-runtime"
Expand Down Expand Up @@ -53,7 +52,7 @@ func (v *VClusterOps) RestartNode(ctx context.Context, opts ...restartnode.Optio
}

func (v *VClusterOps) genRestartNodeOptions(s *restartnode.Parms, certs *HTTPSCerts) *vops.VRestartNodesOptions {
su := vapi.SuperUser
su := v.VDB.GetVerticaUser()
honorUserInput := true
opts := vops.VRestartNodesOptions{
DatabaseOptions: vops.DatabaseOptions{
Expand Down
2 changes: 1 addition & 1 deletion pkg/vadmin/start_db_vc.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func (v *VClusterOps) genStartDBOptions(s *startdb.Parms, certs *HTTPSCerts) (vo
opts.Key = certs.Key
opts.Cert = certs.Cert
opts.CaCert = certs.CaCert
*opts.UserName = vapi.SuperUser
*opts.UserName = v.VDB.GetVerticaUser()
opts.Password = &v.Password
*opts.HonorUserInput = true

Expand Down
3 changes: 1 addition & 2 deletions pkg/vadmin/stop_db_vc.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (

vops "github.com/vertica/vcluster/vclusterops"
"github.com/vertica/vcluster/vclusterops/vstruct"
vapi "github.com/vertica/vertica-kubernetes/api/v1"
"github.com/vertica/vertica-kubernetes/pkg/net"
"github.com/vertica/vertica-kubernetes/pkg/vadmin/opts/stopdb"
)
Expand Down Expand Up @@ -58,7 +57,7 @@ func (v *VClusterOps) genStopDBOptions(s *stopdb.Parms) vops.VStopDatabaseOption
opts.IsEon = vstruct.MakeNullableBool(v.VDB.IsEON())

// auth options
*opts.UserName = vapi.SuperUser
*opts.UserName = v.VDB.GetVerticaUser()
opts.Password = &v.Password
*opts.HonorUserInput = true

Expand Down
Loading