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

Fixes nil pointer on workgroup tags #69

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

veqryn
Copy link

@veqryn veqryn commented Nov 12, 2024

This PR fixes the following panic:
If you create an athena Config with a named athena workgroup, with the default workgroup tags, it will give a nil pointer panic when it tries to create the workgroup (and also if your aws permissions are incorrect).

This is because when QueryContext goes to get the current workgroup, even if the workgroup has the default tags, it will initialize a new workgroup with nil for tags, resulting in a nil pointer error when the code later tries to get the tags.

runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x8 pc=0xf8b325]

goroutine 1 [running]:
github.com/uber/athenadriver/go.(*WGTags).Get(...)
        /go/src/vendor/github.com/uber/athenadriver/go/wgtag.go:44
github.com/uber/athenadriver/go.(*Workgroup).CreateWGRemotely(0xc0009bd028, {0x1a2cbc8, 0xc0001b4510})
        /go/src/vendor/github.com/uber/athenadriver/go/wg.go:81 +0x25
github.com/uber/athenadriver/go.(*Connection).QueryContext(0xc0008046a0, {0x19fd430, 0x2520360}, {0x16d43f6?, 0xa?}, {0x2520360, 0x0, 0x30?})
        /go/src/vendor/github.com/uber/athenadriver/go/connection.go:279 +0xbc5
database/sql.ctxDriverQuery({0x19fd430?, 0x2520360?}, {0x7fffb83282b8?, 0xc0008046a0?}, {0x0?, 0x0?}, {0x16d43f6?, 0x0?}, {0x2520360, 0x0, ...})
        /usr/local/go/src/database/sql/ctxutil.go:48 +0xd7
database/sql.(*DB).queryDC.func1()
        /usr/local/go/src/database/sql/sql.go:1778 +0x159
database/sql.withLock({0x19f3cb0, 0xc00081a500}, 0xc00062d428)
        /usr/local/go/src/database/sql/sql.go:3566 +0x71
database/sql.(*DB).queryDC(0x1?, {0x19fd430, 0x2520360}, {0x0, 0x0}, 0xc00081a500, 0xc000806670, {0x16d43f6, 0xa}, {0x0, ...})
        /usr/local/go/src/database/sql/sql.go:1773 +0x1de
database/sql.(*DB).query(0xc0001bf2b0, {0x19fd430, 0x2520360}, {0x16d43f6, 0xa}, {0x0, 0x0, 0x0}, 0x88?)
        /usr/local/go/src/database/sql/sql.go:1756 +0xfc
database/sql.(*DB).QueryContext.func1(0x0?)
        /usr/local/go/src/database/sql/sql.go:1734 +0x4f
database/sql.(*DB).retry(0xc000804640?, 0xc0009bd608)
        /usr/local/go/src/database/sql/sql.go:1568 +0x42
database/sql.(*DB).QueryContext(0x9?, {0x19fd430?, 0x2520360?}, {0x16d43f6?, 0x0?}, {0x0?, 0xc00062d6c8?, 0x4123a5?})
        /usr/local/go/src/database/sql/sql.go:1733 +0xc5
database/sql.(*DB).QueryRowContext(...)
        /usr/local/go/src/database/sql/sql.go:1834
database/sql.(*DB).QueryRow(0x16d3100?, {0x16d43f6?, 0xc000146500?}, {0x0?, 0x2?, 0x6?})
        /usr/local/go/src/database/sql/sql.go:1848 +0x45

Reproduce-able setup:
(Just make sure the workgroup doesn't exist yet or you have bad credentials.)

	athenaCfg := athenadrv.NewNoOpsConfig()
	athenaCfg.SetResultPollIntervalSeconds(athenadrv.PoolInterval)
	athenaCfg.SetWGRemoteCreationAllowed(true)
	athenaCfg.SetDB(athenaDatabase)

	err := athenaCfg.SetRegion(awsRegion)
	if err != nil {
		return nil, fmt.Errorf("unable to set athena region: %w", err)
	}

	err = athenaCfg.SetOutputBucket(fmt.Sprintf("s3://%s/%s", athenaResultsBucket, athenaResultsPath))
	if err != nil {
		return nil, fmt.Errorf("unable to set athena output bucket: %w", err)
	}

	err = athenaCfg.SetWorkGroup(athenadrv.NewDefaultWG(athenaWorkgroup, &athenav1.WorkGroupConfiguration{
		EnforceWorkGroupConfiguration:   aws.Bool(false),
		PublishCloudWatchMetricsEnabled: aws.Bool(true),
	}, nil))
	if err != nil {
		return nil, fmt.Errorf("unable to set athena workgroup: %w", err)
	}

	athenaDB, err := sql.Open(athenadrv.DriverName, athenaCfg.Stringify())

@CLAassistant
Copy link

CLAassistant commented Nov 12, 2024

CLA assistant check
All committers have signed the CLA.

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

Successfully merging this pull request may close these issues.

2 participants