Stream: duplicated conditions in getProvider methods from KinesisSink.scala and KinesisEnrich.scala

[chuwy]

Projects:

• Collectors
• Enrich

Version: Master

Expected behavior: Log warning message in case of accessKey or secretKey not being set

Actual behavior: Never logging anything regarding accessKey or secretKey not being set

Steps to reproduce:

1. Call methods KinesisSink.getProvider() or KinesisEnrich.getProvider() with only one of the 2 configuration key filled
2. Don't get any warning

---

Hello there,

I'm a developer at SonarSource (the company developing SonarCloud), and I'm currently working on improving our Scala static analyzer. In order to do so, I'm using your project (among other open source scala-based projects) to dogfood and test the robustness of our rules.

While reworking the way we handle the Scala match expressions, some of our rules started raising issues on two small bugs in your code, and I thought you would like to ear about them.

For the following files, both cases are strictly identical:

• Collectors: KinesisSink.scala; L86 && L88
• Enrich: KinesisEnrich.scala: L298 && L300

And to give an explicit view to the code (KinesisSink) with my comment:

  /** Create an aws credentials provider through env variables and iam. */
  private def getProvider(awsConfig: AWSConfig): \/[Throwable, AWSCredentialsProvider] = {
    def isDefault(key: String): Boolean = key == "default"
    def isIam(key: String): Boolean = key == "iam"
    def isEnv(key: String): Boolean = key == "env"

    ((awsConfig.accessKey, awsConfig.secretKey) match {
      case (a, s) if isDefault(a) && isDefault(s) =>
        new DefaultAWSCredentialsProviderChain().right
      case (a, s) if isDefault(a) || isDefault(s) =>
        "accessKey and secretKey must both be set to 'default' or neither".left
      case (a, s) if isIam(a) && isIam(s) =>
        InstanceProfileCredentialsProvider.getInstance().right
      // FIXME The following condition is strictly equal to the previous one, 
      // and will never be executed. It should have been a OR.
      case (a, s) if isIam(a) && isIam(s) =>
        "accessKey and secretKey must both be set to 'iam' or neither".left
      case (a, s) if isEnv(a) && isEnv(s) =>
        new EnvironmentVariableCredentialsProvider().right
      case (a, s) if isEnv(a) || isEnv(s) =>
        "accessKey and secretKey must both be set to 'env' or neither".left
      case _ => new AWSStaticCredentialsProvider(
        new BasicAWSCredentials(awsConfig.accessKey, awsConfig.secretKey)).right
    }).leftMap(new IllegalArgumentException(_))
  }

As a consequence, it seems to me that you are never going to be warned when one of the two keys is not set.

Note that if you try to analyze your project with SonarCloud (which is completely free for open source projects), these issues won't be detected yet. They will only be visible with the next version of our analyzer (also free and open source), which will be deployed in a few days.

Cheers, Michael

[chuwy]

Migrated from https://github.com/snowplow/snowplow/issues/4008 (comments are auto-generated)