diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 618c4264..7ca1c94c 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -49,7 +49,7 @@ jobs: baton-principal-type: user bad-credentials: DB_PASSWORD=invalid - name: Run account provisioning tests - uses: ConductorOne/github-workflows/actions/account-provisioning@v3 + uses: ConductorOne/github-workflows/actions/account-provisioning@v4 with: connector: ./baton-sql account-email: robert.tables2@example.com diff --git a/docs/db2.md b/docs/db2.md index 751c3709..9ad7e8fc 100644 --- a/docs/db2.md +++ b/docs/db2.md @@ -110,12 +110,20 @@ Query parameters are forwarded as additional DB2 connection keywords (`HOSTNAME`, `DATABASE`, `PORT`, `PROTOCOL`, `UID`, `PWD`) are rejected — use the native form below for full control. -DB2's native form is also accepted as-is: +DB2's native form is also accepted as-is (ODBC keywords are case-insensitive and may carry +spaces after each `;`): ``` HOSTNAME=localhost;PORT=50000;DATABASE=TESTDB;UID=db2inst1;PWD=pass123;PROTOCOL=TCPIP ``` +The native form is self-contained: it already carries the host, port, credentials, params and +target database. It is therefore mutually exclusive with the structured `connect` fields +(`host`, `port`, `user`, `password`, `params`) and with a per-database override (`connect.database` +or the `databases` block for multi-database sync). Combining them is rejected with an explicit +error rather than silently ignoring the extra settings, so use the `db2://` URL form when you +need multi-database discovery or want to supply fields separately. + ## Writing a Db2 spec Db2 needs two things in every spec. Other engines need them only in spots (Oracle folds diff --git a/pkg/bsql/offline_validate.go b/pkg/bsql/offline_validate.go index 04f4dad5..4ba441d6 100644 --- a/pkg/bsql/offline_validate.go +++ b/pkg/bsql/offline_validate.go @@ -5,6 +5,8 @@ import ( "fmt" "net/url" "strings" + + "github.com/conductorone/baton-sql/pkg/database/db2" ) // OfflineValidate performs YAML-level structural checks without opening a DB or @@ -102,6 +104,12 @@ func resolveConnectScheme(c *DatabaseConfig) (string, error) { if dsn == "" { return "", errors.New("connect: scheme or dsn is required") } + // A native DB2 DSN (HOSTNAME=...;DATABASE=...) carries no scheme prefix. Classify it + // via the shared detector so this check matches pkg/database's routing and does not + // misread a "://" inside a value as a scheme. + if db2.IsNativeDSN(dsn) { + return "db2", nil + } // Placeholders like postgres://${HOST}/db — peel scheme before parse when possible. if idx := strings.Index(dsn, "://"); idx > 0 { return strings.ToLower(dsn[:idx]), nil diff --git a/pkg/bsql/offline_validate_test.go b/pkg/bsql/offline_validate_test.go index 430e0be3..15656400 100644 --- a/pkg/bsql/offline_validate_test.go +++ b/pkg/bsql/offline_validate_test.go @@ -124,6 +124,27 @@ func TestRejectNonV1_NonPostgresScheme(t *testing.T) { require.Contains(t, strings.ToLower(err.Error()), "postgres") } +func TestRejectNonV1_NativeDB2DSNRejectedAsDB2(t *testing.T) { + // A native DB2 DSN carries no scheme prefix; it must be classified as "db2" (via the + // shared detector), so v1 rejects it with the scheme message rather than the confusing + // "scheme missing from dsn". Also guards against a "://" inside a value misclassifying it. + for _, dsn := range []string{ + "HOSTNAME=localhost;PORT=50000;DATABASE=TESTDB;UID=u;PWD=p;PROTOCOL=TCPIP", + "HOSTNAME=localhost;DATABASE=TESTDB;PWD=my://secret", + } { + cfg, err := Parse([]byte(minimalPostgresYAML())) + require.NoError(t, err) + cfg.Connect.Scheme = "" + cfg.Connect.DSN = dsn + s, err := resolveConnectScheme(&cfg.Connect) + require.NoError(t, err) + require.Equal(t, "db2", s) + err = RejectNonV1ProductFeatures(cfg) + require.Error(t, err) + require.Contains(t, err.Error(), "db2") + } +} + func TestRejectNonV1_PostgresqlAliasRejected(t *testing.T) { cfg, err := Parse([]byte(minimalPostgresYAML())) require.NoError(t, err) diff --git a/pkg/connector/connector.go b/pkg/connector/connector.go index eabbf346..129585eb 100644 --- a/pkg/connector/connector.go +++ b/pkg/connector/connector.go @@ -98,7 +98,7 @@ func (c *Connector) Validate(ctx context.Context) (annotations.Annotations, erro for name, db := range c.dbs { if err := db.PingContext(ctx); err != nil { if authErr := database.AuthError(err); authErr != nil { - return nil, authErr + return nil, fmt.Errorf("database %q: %w", name, authErr) } return nil, fmt.Errorf("database %q ping failed: %w", name, err) } diff --git a/pkg/database/autherror.go b/pkg/database/autherror.go index 96593707..08b1c728 100644 --- a/pkg/database/autherror.go +++ b/pkg/database/autherror.go @@ -4,32 +4,38 @@ import ( "errors" "strings" + "github.com/conductorone/baton-sdk/pkg/uhttp" + "github.com/conductorone/baton-sql/pkg/database/db2" "github.com/go-sql-driver/mysql" "google.golang.org/grpc/codes" - "google.golang.org/grpc/status" ) const mysqlAccessDenied = 1045 -// AuthError returns an Unauthenticated gRPC status when err is a database -// authentication/authorization failure, or nil otherwise. SQLSTATE class 28 -// ("invalid authorization") is the ANSI code drivers report on bad credentials -// (Postgres/Redshift/Vertica/etc. surface it via SQLState()); MySQL is the -// exception, reporting error 1045 with no SQLSTATE. +// AuthError wraps err in an Unauthenticated gRPC status when it is a database +// authentication failure, or returns nil otherwise. Each driver surfaces bad +// credentials differently: SQLSTATE class 28 via a SQLState() method (Postgres/ +// Redshift), MySQL error 1045 with no SQLSTATE, and DB2 in a driver-specific +// field (see db2.IsAuthError). Drivers that expose SQLSTATE only as a struct field +// (Vertica) or not at all (Oracle, MSSQL, SAP HDB) are not covered and fall through +// to a generic ping error. Wrapping keeps the original error for errors.As. func AuthError(err error) error { - if err == nil { + if err == nil || !isAuthFailure(err) { return nil } + return uhttp.WrapErrors(codes.Unauthenticated, "database authentication failed", err) +} +func isAuthFailure(err error) bool { var sqlState interface{ SQLState() string } if errors.As(err, &sqlState) && strings.HasPrefix(sqlState.SQLState(), "28") { - return status.Error(codes.Unauthenticated, "database authentication failed") + return true } var myErr *mysql.MySQLError if errors.As(err, &myErr) && myErr.Number == mysqlAccessDenied { - return status.Error(codes.Unauthenticated, "database authentication failed") + return true } - return nil + return db2.IsAuthError(err) } diff --git a/pkg/database/database.go b/pkg/database/database.go index 005164ef..d8a88e56 100644 --- a/pkg/database/database.go +++ b/pkg/database/database.go @@ -18,6 +18,8 @@ import ( "github.com/conductorone/baton-sql/pkg/database/postgres" "github.com/conductorone/baton-sql/pkg/database/sqlserver" "github.com/conductorone/baton-sql/pkg/database/vertica" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" ) var DSNREnvRegex = regexp.MustCompile(`\$\{([A-Za-z0-9_]+)\}`) @@ -361,6 +363,9 @@ func ResolveDatabaseName(opts ConnectOptions) string { return expanded } } + if _, database, isNativeDB2, err := nativeDB2DSN(opts); err == nil && isNativeDB2 { + return database + } parsedUrl, err := buildConnectionURL(opts) if err != nil || parsedUrl == nil { return "" @@ -368,6 +373,16 @@ func ResolveDatabaseName(opts ConnectOptions) string { return strings.TrimPrefix(parsedUrl.Path, "/") } +// hasStructuredConnectFields reports whether opts carries any structured connect +// field that a native DB2 DSN would make redundant. A native DSN is self-contained; +// combining it with these (or a per-database override) silently drops them, so the +// caller rejects the combination. Scheme is excluded: "db2" alongside a native DSN +// is a supported, explicit hint. +func hasStructuredConnectFields(opts ConnectOptions) bool { + return opts.Host != "" || opts.Port != "" || opts.User != "" || + opts.Password != "" || opts.Database != "" || len(opts.Params) > 0 +} + // ConnectMany opens one *sql.DB per name in dbNames. On any per-database failure, // every handle opened so far is closed before returning the error. func ConnectMany(ctx context.Context, opts ConnectOptions, dbNames []string) (map[string]*sql.DB, DbEngine, error) { @@ -404,13 +419,41 @@ func ConnectMany(ctx context.Context, opts ConnectOptions, dbNames []string) (ma } func Connect(ctx context.Context, opts ConnectOptions) (*sql.DB, DbEngine, error) { + // A native DB2 DSN is an opaque ODBC keyword=value string, not a URL. Routing it + // through buildConnectionURL corrupts it (url.Parse/.String mangles the opaque form), + // so hand it to the driver verbatim. See docs/db2.md. + nativeDSN, _, isNativeDB2, err := nativeDB2DSN(opts) + if err != nil { + return nil, Unknown, err + } + if isNativeDB2 { + // A native DSN already carries host, port, credentials, params and the target + // database. Structured fields or a per-database override (set directly, or by + // ConnectMany for databases.static / discovery_query) would be silently dropped + // on the verbatim path, so reject the combination instead of connecting to the + // wrong database. See docs/db2.md. + if hasStructuredConnectFields(opts) { + return nil, Unknown, status.Error(codes.InvalidArgument, + "native DB2 DSN is self-contained and cannot be combined with structured "+ + "connect fields (host, port, user, password, params) or a per-database "+ + "override (connect.database, databases); put every setting in the DSN or "+ + "use the db2:// URL form", + ) + } + db, err := db2.Connect(ctx, nativeDSN) + if err != nil { + return nil, Unknown, err + } + return db, DB2, nil + } + parsedDsn, err := buildConnectionURL(opts) if err != nil { return nil, Unknown, err } if parsedDsn.Scheme == "" { - return nil, Unknown, errors.New("database scheme must be specified in DSN or configuration") + return nil, Unknown, status.Error(codes.InvalidArgument, "database scheme must be specified in DSN or configuration") } switch parsedDsn.Scheme { @@ -468,6 +511,43 @@ func Connect(ctx context.Context, opts ConnectOptions) (*sql.DB, DbEngine, error } } +// nativeDB2DSN reports whether opts carries a native DB2 DSN: an opaque ODBC +// keyword=value string (e.g. "HOSTNAME=...;DATABASE=...") rather than a db2:// URL. +// When it does, the env-expanded string (for verbatim handoff to the driver) and its +// DATABASE value are returned. The scheme, when set, must be db2; a URL-shaped DSN or +// foreign scheme is left to the normal URL path. Detection is db2.ParseNativeDSN, shared +// with convertToDB2DSN's passthrough, so one pass yields both facts without re-splitting. +func nativeDB2DSN(opts ConnectOptions) (string, string, bool, error) { + if opts.DSN == "" { + return "", "", false, nil + } + lookup := opts.resolveLookup() + + scheme, err := expandValue(opts.Scheme, lookup) + if err != nil { + return "", "", false, err + } + if scheme != "" && scheme != "db2" { + return "", "", false, nil + } + + dsn, err := expandValue(opts.DSN, lookup) + if err != nil { + return "", "", false, err + } + if _, native := db2.ParseNativeDSN(dsn); !native { + return "", "", false, nil + } + // Confirmed native: re-expand with keyword-injection validation. The expansion above + // only decides routing; the driver gets this string verbatim. + safeDSN, err := expandNativeDSN(opts.DSN, lookup) + if err != nil { + return "", "", false, err + } + database, _ := db2.ParseNativeDSN(safeDSN) + return safeDSN, database, true, nil +} + func buildConnectionURL(opts ConnectOptions) (*url.URL, error) { var ( parsedUrl *url.URL @@ -605,3 +685,39 @@ func expandValue(s string, lookup LookupFunc) (string, error) { } return s, nil } + +// expandNativeDSN expands ${KEY} placeholders in a native DB2 DSN, rejecting any expanded +// value that carries an ODBC keyword separator (; { } =). The db2:// URL path quotes each +// field with quoteDB2Value, but a native DSN is handed to the driver verbatim, so a +// placeholder value here could otherwise inject or override DSN keywords. +func expandNativeDSN(dsn string, lookup LookupFunc) (string, error) { + if !DSNREnvRegex.MatchString(dsn) { + return dsn, nil + } + // A DSN that is a single ${KEY} spanning the whole string is the full value, not a + // field embedded in literal structure, so its separators are legitimate: expand as-is. + if DSNREnvRegex.FindString(dsn) == dsn { + return expandValue(dsn, lookup) + } + if lookup == nil { + lookup = os.LookupEnv + } + var err error + result := DSNREnvRegex.ReplaceAllStringFunc(dsn, func(match string) string { + varName := match[2 : len(match)-1] + value, exists := lookup(varName) + if !exists { + err = errors.Join(err, fmt.Errorf("environment variable %s is not set", varName)) + return match + } + if strings.ContainsAny(value, ";{}=") { + err = errors.Join(err, fmt.Errorf("value for %s must not contain ODBC keyword separators (; { } =)", varName)) + return match + } + return value + }) + if err != nil { + return "", err + } + return result, nil +} diff --git a/pkg/database/db2/autherror_test.go b/pkg/database/db2/autherror_test.go new file mode 100644 index 00000000..29442b84 --- /dev/null +++ b/pkg/database/db2/autherror_test.go @@ -0,0 +1,33 @@ +//go:build db2 + +package db2 + +import ( + "fmt" + "testing" + + "github.com/ibmdb/go_ibm_db" + "github.com/stretchr/testify/require" +) + +func TestIsAuthError(t *testing.T) { + badCreds := &go_ibm_db.Error{Diag: []go_ibm_db.DiagRecord{{State: "28000"}}} + + tests := []struct { + name string + err error + want bool + }{ + {"class 28 bad credentials", badCreds, true}, + {"class 28 wrapped", fmt.Errorf("connect: %w", badCreds), true}, + {"non-auth sqlstate", &go_ibm_db.Error{Diag: []go_ibm_db.DiagRecord{{State: "42501"}}}, false}, + {"no diag records", &go_ibm_db.Error{}, false}, + {"unrelated error", fmt.Errorf("boom"), false}, + {"nil", nil, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, tt.want, IsAuthError(tt.err)) + }) + } +} diff --git a/pkg/database/db2/db2.go b/pkg/database/db2/db2.go index 1b942c65..94a5144c 100644 --- a/pkg/database/db2/db2.go +++ b/pkg/database/db2/db2.go @@ -5,9 +5,11 @@ package db2 import ( "context" "database/sql" + "errors" + "strings" "time" - _ "github.com/ibmdb/go_ibm_db" + "github.com/ibmdb/go_ibm_db" ) // Connect establishes a connection to DB2 database. @@ -35,3 +37,18 @@ func Connect(ctx context.Context, dsn string) (*sql.DB, error) { return db, nil } + +// IsAuthError reports whether err is a DB2 auth failure. go_ibm_db exposes SQLSTATE +// via Error.Diag[].State, not a SQLState() method, so class 28 must be matched here. +func IsAuthError(err error) bool { + var db2Err *go_ibm_db.Error + if !errors.As(err, &db2Err) { + return false + } + for _, rec := range db2Err.Diag { + if strings.HasPrefix(rec.State, "28") { + return true + } + } + return false +} diff --git a/pkg/database/db2/db2_stub.go b/pkg/database/db2/db2_stub.go index 389b6b59..418e2f6f 100644 --- a/pkg/database/db2/db2_stub.go +++ b/pkg/database/db2/db2_stub.go @@ -14,3 +14,8 @@ import ( func Connect(_ context.Context, _ string) (*sql.DB, error) { return nil, errors.New("baton-sql: DB2 support not compiled into this binary; rebuild with -tags db2 (see docs/db2.md)") } + +// IsAuthError is a stub; the DB2 driver error types are unavailable without -tags db2. +func IsAuthError(_ error) bool { + return false +} diff --git a/pkg/database/db2/dsn.go b/pkg/database/db2/dsn.go index 87b7d3ed..c1c41491 100644 --- a/pkg/database/db2/dsn.go +++ b/pkg/database/db2/dsn.go @@ -3,10 +3,105 @@ package db2 import ( "fmt" "net/url" + "regexp" "sort" "strings" ) +// urlSchemeRegex matches a DSN that begins with a URL scheme (e.g. "db2://"). +// Anchored to the start so a native ODBC DSN carrying "://" inside a value +// (e.g. PWD=my://secret) is not misread as a URL. +var urlSchemeRegex = regexp.MustCompile(`^[a-zA-Z][a-zA-Z0-9+.-]*://`) + +// ParseNativeDSN reports whether dsn is DB2's native ODBC keyword=value form (rather +// than a URL) and, when it is, returns its DATABASE value ("" if the DSN omits one). +// The HOSTNAME keyword is the native marker: DATABASE alone does not qualify, because +// generic ODBC/ADO strings from other engines (e.g. "Server=x;Database=y") also carry +// DATABASE and would otherwise be misrouted to the DB2 driver instead of failing with +// the normal scheme-missing error. One pass over the DSN; IsNativeDSN and DSNDatabase +// are thin wrappers so all callers (pkg/database routing, convertToDB2DSN passthrough, +// pkg/bsql offline scheme check) share one decision and cannot drift. ODBC keywords are +// case-insensitive and keyword/value may carry surrounding whitespace, so both are normalized. +func ParseNativeDSN(dsn string) (string, bool) { + if urlSchemeRegex.MatchString(dsn) { + return "", false + } + var database string + native, haveDB := false, false + for _, part := range splitDB2DSN(dsn) { + keyword, value, found := strings.Cut(part, "=") + if !found { + continue + } + keyword = strings.TrimSpace(keyword) + switch { + case strings.EqualFold(keyword, "HOSTNAME"): + native = true + case strings.EqualFold(keyword, "DATABASE"): + if !haveDB { // first DATABASE= wins + value = strings.TrimSpace(value) + if strings.HasPrefix(value, "{") && strings.HasSuffix(value, "}") { + value = value[1 : len(value)-1] + } + database, haveDB = value, true + } + } + } + return database, native +} + +// IsNativeDSN reports whether dsn is DB2's native ODBC keyword=value form. +func IsNativeDSN(dsn string) bool { + _, native := ParseNativeDSN(dsn) + return native +} + +// DSNDatabase returns the DATABASE keyword value from a native DB2 DSN, or "" if absent. +func DSNDatabase(dsn string) string { + database, _ := ParseNativeDSN(dsn) + return database +} + +// splitDB2DSN splits a native DB2 DSN on ';', ignoring separators inside {} quoting. +// A '{' quotes only when it starts a value (right after '=', across any whitespace) AND +// is closed by a later '}'. A '{' elsewhere, or one left unterminated, is literal, so +// PWD=p{q keeps the following ';' and an unclosed '{' does not swallow the rest of the +// DSN (its HOSTNAME/DATABASE markers stay visible and the malformed value reaches the +// driver's own error rather than a silent misroute). +func splitDB2DSN(dsn string) []string { + var parts []string + start := 0 + braced := false // inside a {...} quoted value + atValueStart := false // at a value position (right after '=', across whitespace) outside braces + for i := 0; i < len(dsn); i++ { + switch dsn[i] { + case '}': + braced = false + atValueStart = false + case '{': + if atValueStart && strings.IndexByte(dsn[i:], '}') != -1 { + braced = true + } + atValueStart = false + case '=': + if !braced { + atValueStart = true + } + case ';': + if !braced { + parts = append(parts, dsn[start:i]) + start = i + 1 + } + atValueStart = false + case ' ', '\t': + // keep atValueStart so "DATABASE= {my;db}" still brace-detects. + default: + atValueStart = false + } + } + return append(parts, dsn[start:]) +} + // Keywords derived from the URL itself; query parameters may not override them. // Anyone needing full control over these can pass a native DB2 DSN instead. var reservedDSNKeywords = map[string]bool{ @@ -33,9 +128,9 @@ func quoteDB2Value(v string) (string, error) { // convertToDB2DSN converts URL format to DB2 DSN format. func convertToDB2DSN(dsn string) (string, error) { - // If it's already in DB2 format (contains HOSTNAME= or DATABASE=), return as-is. - // URL-format DSNs are exempt from this check so those markers may appear in credentials. - if !strings.HasPrefix(dsn, "db2://") && (strings.Contains(dsn, "HOSTNAME=") || strings.Contains(dsn, "DATABASE=")) { + // If it's already in DB2's native keyword=value format, return as-is. + // URL-format DSNs are exempt so those markers may appear in credentials. + if IsNativeDSN(dsn) { return dsn, nil } diff --git a/pkg/database/db2/dsn_test.go b/pkg/database/db2/dsn_test.go index 9ab7956a..a3c0b776 100644 --- a/pkg/database/db2/dsn_test.go +++ b/pkg/database/db2/dsn_test.go @@ -28,6 +28,16 @@ func TestConvertToDB2DSN(t *testing.T) { dsn: "HOSTNAME=dbhost;PORT=50000;DATABASE=testdb;UID=user;PWD=pass", want: "HOSTNAME=dbhost;PORT=50000;DATABASE=testdb;UID=user;PWD=pass", }, + { + name: "lowercase native dsn passed through", + dsn: "hostname=dbhost;port=50000;database=testdb;uid=user;pwd=pass", + want: "hostname=dbhost;port=50000;database=testdb;uid=user;pwd=pass", + }, + { + name: "native dsn with whitespace passed through", + dsn: "HOSTNAME=dbhost; DATABASE=testdb; UID=user", + want: "HOSTNAME=dbhost; DATABASE=testdb; UID=user", + }, { name: "wrong scheme", dsn: "postgres://dbhost/testdb", @@ -102,3 +112,58 @@ func TestConvertToDB2DSN(t *testing.T) { }) } } + +func TestIsNativeDSN(t *testing.T) { + tests := []struct { + name string + dsn string + want bool + }{ + {name: "native markers", dsn: "HOSTNAME=h;DATABASE=X", want: true}, + {name: "lowercase keywords", dsn: "hostname=h;database=x", want: true}, + {name: "whitespace after separator", dsn: "HOSTNAME=h; DATABASE=X", want: true}, + {name: "db2 url", dsn: "db2://u:p@h:50000/db", want: false}, + {name: "postgres url", dsn: "postgres://h/db", want: false}, + {name: "value carrying :// is not a url", dsn: "HOSTNAME=h;PWD=my://secret", want: true}, + {name: "space before the =", dsn: "HOSTNAME = h;DATABASE=X", want: true}, + // DATABASE without HOSTNAME is a generic ODBC/ADO shape (e.g. MSSQL), not native DB2. + {name: "database without hostname is not native", dsn: "Server=x;Database=y;User Id=u", want: false}, + // HOSTNAME appears only inside a braced PWD value, so the brace-aware split keeps it + // as one PWD part: not a native marker. + {name: "hostname marker only inside braced value", dsn: "UID=u;PWD={x;HOSTNAME=y}", want: false}, + // Unterminated '{' is literal, so the ';' still splits and HOSTNAME= stays visible; + // the malformed value then reaches the driver instead of silently misrouting. + {name: "unterminated brace keeps marker visible", dsn: "PWD={oops;HOSTNAME=h", want: true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, tt.want, IsNativeDSN(tt.dsn)) + }) + } +} + +func TestDSNDatabase(t *testing.T) { + tests := []struct { + name string + dsn string + want string + }{ + {name: "plain", dsn: "HOSTNAME=h;DATABASE=TESTDB;UID=u", want: "TESTDB"}, + {name: "braced value with semicolon", dsn: "HOSTNAME=h;DATABASE={my;db}", want: "my;db"}, + {name: "lowercase", dsn: "hostname=h;database=testdb", want: "testdb"}, + {name: "whitespace before keyword", dsn: "HOSTNAME=h; DATABASE=TESTDB", want: "TESTDB"}, + {name: "space after the =", dsn: "HOSTNAME=h;DATABASE= TESTDB", want: "TESTDB"}, + {name: "space before the =", dsn: "HOSTNAME=h;DATABASE = TESTDB", want: "TESTDB"}, + // Space between '=' and a braced value must still brace-detect, else the ';' + // inside the braces splits and the database name comes back truncated. + {name: "space before braced value", dsn: "HOSTNAME=h;DATABASE= {my;db}", want: "my;db"}, + // A literal '{' mid-value (not ODBC quoting) must not swallow the following ';'. + {name: "unquoted brace in earlier value", dsn: "HOSTNAME=h;PWD=p{q;DATABASE=TESTDB", want: "TESTDB"}, + {name: "absent", dsn: "HOSTNAME=h;UID=u", want: ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, tt.want, DSNDatabase(tt.dsn)) + }) + } +} diff --git a/pkg/database/native_db2_dsn_test.go b/pkg/database/native_db2_dsn_test.go new file mode 100644 index 00000000..b5afda9c --- /dev/null +++ b/pkg/database/native_db2_dsn_test.go @@ -0,0 +1,151 @@ +package database + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestNativeDB2DSN(t *testing.T) { + const native = "HOSTNAME=localhost;PORT=50000;DATABASE=TESTDB;UID=db2inst1;PWD=pass123;PROTOCOL=TCPIP" + + lookup := func(m map[string]string) LookupFunc { + return func(k string) (string, bool) { v, ok := m[k]; return v, ok } + } + + tests := []struct { + name string + opts ConnectOptions + wantDSN string + wantOk bool + wantErr string + }{ + {name: "native form no scheme", opts: ConnectOptions{DSN: native}, wantDSN: native, wantOk: true}, + {name: "native form with scheme db2", opts: ConnectOptions{DSN: native, Scheme: "db2"}, wantDSN: native, wantOk: true}, + // DATABASE without HOSTNAME is a generic ODBC/ADO shape, not native DB2: it must + // fall through to the normal scheme check rather than route to the DB2 driver. + {name: "database marker only is not native", opts: ConnectOptions{DSN: "DATABASE=TESTDB;HOST=x"}, wantDSN: "", wantOk: false}, + { + name: "lowercase keywords", + opts: ConnectOptions{DSN: "hostname=h;port=50000;database=X;uid=u;pwd=p"}, + wantDSN: "hostname=h;port=50000;database=X;uid=u;pwd=p", + wantOk: true, + }, + { + name: "whitespace after separators", + opts: ConnectOptions{DSN: "HOSTNAME=h; DATABASE=X; UID=u"}, + wantDSN: "HOSTNAME=h; DATABASE=X; UID=u", + wantOk: true, + }, + { + name: "value containing :// is not a url", + opts: ConnectOptions{DSN: "HOSTNAME=h;DATABASE=X;PWD=my://secret"}, + wantDSN: "HOSTNAME=h;DATABASE=X;PWD=my://secret", + wantOk: true, + }, + { + name: "native form with placeholders", + opts: ConnectOptions{ + DSN: "HOSTNAME=${DB_HOST};PORT=50000;DATABASE=${DB_NAME};UID=u;PWD=p", + Lookup: lookup(map[string]string{"DB_HOST": "h", "DB_NAME": "d"}), + }, + wantDSN: "HOSTNAME=h;PORT=50000;DATABASE=d;UID=u;PWD=p", + wantOk: true, + }, + { + name: "scheme placeholder expands to db2", + opts: ConnectOptions{DSN: native, Scheme: "${SCH}", Lookup: lookup(map[string]string{"SCH": "db2"})}, + wantDSN: native, + wantOk: true, + }, + {name: "db2 url form", opts: ConnectOptions{DSN: "db2://u:p@h:50000/db"}, wantOk: false}, + {name: "postgres url form", opts: ConnectOptions{DSN: "postgres://h/db"}, wantOk: false}, + {name: "native markers but foreign scheme", opts: ConnectOptions{DSN: native, Scheme: "postgres"}, wantOk: false}, + {name: "url with database marker in query", opts: ConnectOptions{DSN: "db2://h:50000/db?DATABASE=x"}, wantOk: false}, + {name: "empty dsn", opts: ConnectOptions{}, wantOk: false}, + { + name: "unset placeholder errors", + opts: ConnectOptions{DSN: "HOSTNAME=${MISSING};DATABASE=d", Lookup: lookup(map[string]string{})}, + wantErr: "MISSING", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotDSN, _, gotOk, err := nativeDB2DSN(tt.opts) + if tt.wantErr != "" { + require.ErrorContains(t, err, tt.wantErr) + return + } + require.NoError(t, err) + require.Equal(t, tt.wantOk, gotOk) + require.Equal(t, tt.wantDSN, gotDSN) + }) + } +} + +func TestResolveDatabaseNameNativeDB2(t *testing.T) { + // The native form must resolve the same database name as the equivalent db2:// URL, + // so resource IDs stay stable across the two DSN forms. + tests := []struct { + name string + opts ConnectOptions + want string + }{ + { + name: "native form", + opts: ConnectOptions{DSN: "HOSTNAME=h;PORT=50000;DATABASE=TESTDB;UID=u;PWD=p;PROTOCOL=TCPIP"}, + want: "TESTDB", + }, + { + name: "native form braced database", + opts: ConnectOptions{DSN: "HOSTNAME=h;DATABASE={my;db};UID=u"}, + want: "my;db", + }, + { + name: "lowercase database keyword", + opts: ConnectOptions{DSN: "hostname=h;database=testdb;uid=u"}, + want: "testdb", + }, + { + name: "whitespace before database keyword", + opts: ConnectOptions{DSN: "HOSTNAME=h; DATABASE=TESTDB; UID=u"}, + want: "TESTDB", + }, + { + name: "equivalent url form", + opts: ConnectOptions{DSN: "db2://u:p@h:50000/TESTDB"}, + want: "TESTDB", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, tt.want, ResolveDatabaseName(tt.opts)) + }) + } +} + +func TestExpandNativeDSN(t *testing.T) { + lookup := func(k string) (string, bool) { + m := map[string]string{"H": "dbhost", "PW": "secret", "BAD": "x;DATABASE=other"} + v, ok := m[k] + return v, ok + } + + got, err := expandNativeDSN("HOSTNAME=${H};PWD=${PW}", lookup) + require.NoError(t, err) + require.Equal(t, "HOSTNAME=dbhost;PWD=secret", got) + + // A placeholder value carrying ODBC separators can't inject extra keywords. + _, err = expandNativeDSN("HOSTNAME=${H};PWD=${BAD}", lookup) + require.ErrorContains(t, err, "ODBC keyword separators") + + // A single ${KEY} spanning the whole DSN is the full value, so its separators are kept. + whole := func(k string) (string, bool) { return "HOSTNAME=h;DATABASE=d", k == "DSN" } + got, err = expandNativeDSN("${DSN}", whole) + require.NoError(t, err) + require.Equal(t, "HOSTNAME=h;DATABASE=d", got) + + _, err = expandNativeDSN("HOSTNAME=${MISSING}", lookup) + require.ErrorContains(t, err, "is not set") +} diff --git a/pkg/database/native_db2_route_test.go b/pkg/database/native_db2_route_test.go new file mode 100644 index 00000000..f782cbee --- /dev/null +++ b/pkg/database/native_db2_route_test.go @@ -0,0 +1,67 @@ +//go:build !db2 + +package database + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" +) + +// A native DB2 DSN set through config must reach the DB2 driver, not be rejected by the +// URL builder. On a default (non-db2) build that means Connect returns the "not compiled" +// stub error, never the "scheme must be specified" / "database name is required" errors +// the URL builder raises for an opaque DSN. +func TestConnectNativeDB2DSNReachesDriver(t *testing.T) { + const native = "HOSTNAME=localhost;PORT=50000;DATABASE=TESTDB;UID=db2inst1;PWD=pass123;PROTOCOL=TCPIP" + + for _, tt := range []struct { + name string + opts ConnectOptions + }{ + {name: "no scheme", opts: ConnectOptions{DSN: native}}, + {name: "scheme db2", opts: ConnectOptions{DSN: native, Scheme: "db2"}}, + } { + t.Run(tt.name, func(t *testing.T) { + _, _, err := Connect(context.Background(), tt.opts) + require.Error(t, err) + require.ErrorContains(t, err, "DB2 support not compiled") + require.NotContains(t, err.Error(), "scheme must be specified") + require.NotContains(t, err.Error(), "database name is required") + }) + } +} + +// A native DSN already carries every connection setting, so pairing it with structured +// connect fields or a per-database override must be rejected up front (before the driver +// stub), never silently dropped. This also covers the multi-database path, where +// ConnectMany sets perOpts.Database per name. +func TestConnectNativeDB2DSNRejectsStructuredFields(t *testing.T) { + const native = "HOSTNAME=localhost;PORT=50000;DATABASE=TESTDB;UID=db2inst1;PWD=pass123;PROTOCOL=TCPIP" + + for _, tt := range []struct { + name string + opts ConnectOptions + }{ + {name: "database override", opts: ConnectOptions{DSN: native, Database: "OTHERDB"}}, + {name: "host", opts: ConnectOptions{DSN: native, Host: "elsewhere"}}, + {name: "port", opts: ConnectOptions{DSN: native, Port: "50001"}}, + {name: "user", opts: ConnectOptions{DSN: native, User: "someone"}}, + {name: "password", opts: ConnectOptions{DSN: native, Password: "secret"}}, + {name: "params", opts: ConnectOptions{DSN: native, Params: map[string]string{"SECURITY": "SSL"}}}, + } { + t.Run(tt.name, func(t *testing.T) { + _, _, err := Connect(context.Background(), tt.opts) + require.Error(t, err) + require.ErrorContains(t, err, "self-contained") + require.NotContains(t, err.Error(), "DB2 support not compiled") + }) + } + + t.Run("multi-database via ConnectMany", func(t *testing.T) { + _, _, err := ConnectMany(context.Background(), ConnectOptions{DSN: native}, []string{"A", "B"}) + require.Error(t, err) + require.ErrorContains(t, err, "self-contained") + }) +}