Skip to content

fix(bdd): every SDK suite silently falls back to a hardcoded address/credentials when its env var is missing #3883

Description

@chengxilo

Summary

Every BDD step-definition file resolves the server address and root credentials with an "env var or hardcoded default" helper. If the env variable is absent, misspelled, or dropped from a compose overlay, the suite does not fail. Instead, it quietly connects to the default value. Test should be loud when it is broken, we don't need safety or robust here; a fallback turns a configuration bug into either a misleading connection error or, worse, a green run against the wrong server.

The fallbacks also disagree with each other across SDKs, so the same missing variable produces eight different behaviours.

Examples:

pub fn resolve_server_address(role: &str, port: u16) -> String {
match (role.to_lowercase().as_str(), port) {
("leader", 8091) => {
env::var("IGGY_TCP_ADDRESS_LEADER").unwrap_or_else(|_| "iggy-leader:8091".to_string())
}
("follower", 8092) => env::var("IGGY_TCP_ADDRESS_FOLLOWER")
.unwrap_or_else(|_| "iggy-follower:8092".to_string()),
("single", 8090) | (_, 8090) => {
env::var("IGGY_TCP_ADDRESS").unwrap_or_else(|_| "iggy-server:8090".to_string())
}
_ => format!("iggy-server:{}", port),
}
}

func (s basicMessagingSteps) givenRunningServer(ctx context.Context) error {
c := getBasicMessagingCtx(ctx)
addr := os.Getenv("IGGY_TCP_ADDRESS")
if addr == "" {
addr = "127.0.0.1:8090"
}
c.serverAddr = &addr
return nil
}

function server_host(): string
{
return env_or_default('IGGY_HOST', '127.0.0.1');
}
function server_port(): int
{
return (int) env_or_default('IGGY_PORT', '8090');
}

Proposed Change

only use the enviroment varaible, if it's not available, just panic.
For example:

pub fn resolve_server_address(role: &str, port: u16) -> String {
     match (role.to_lowercase().as_str(), port) {
         ("leader", 8091) => required_env("IGGY_TCP_ADDRESS_LEADER"),
         ("follower", 8092) => required_env("IGGY_TCP_ADDRESS_FOLLOWER"),
         ("single", 8090) | (_, 8090) => required_env("IGGY_TCP_ADDRESS"),
         _ => panic!("no address mapping for role '{role}' on port {port}"),
    }
}

fn required_env(name: &str) -> String {
    match env::var(name) {
        Ok(value) if !value.is_empty() => value,
        _ => panic!("{name} must be set; run the suite via scripts/run-bdd-tests.sh"),
    }
}

Can be a good first issue

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions