Problem/Motivation

Hi, thanks for the module! I'm maintaining another module that builds on this one (#2929260: Amazon Polly). When using phpstan to lint the code I came up against an issue where this module's docs weren't entirely accurate (eg. sometimes the doc mentions it might return false on failure, but doesn't indicate that in the actual @return).

Steps to reproduce

Run phpstan at level 3.

Proposed resolution

Fix phpstan level 3 issues and set the variable _PHPSTAN_LEVEL to 3 in .gitlab-ci.yml. We need to wait for #3609096: Error when using AWS profiles with an assumed role and STS AssumeRole fails to land to do that. Also ignore the errors that are fixed by that issue:

 ------ ------------------------------------------------------------------------------ 
  Line   src/Entity/Profile.php                                                        
 ------ ------------------------------------------------------------------------------ 
  347    Caught class Drupal\aws\Entity\Exception not found.                           
         🪪  class.notFound                                                            
  348    Call to method getMessage() on an unknown class Drupal\aws\Entity\Exception.  
         🪪  class.notFound                                                            
 ------ ------------------------------------------------------------------------------ 

Remaining tasks

There's a remaining issue: Method Drupal\aws\Entity\Form\ProfileDeleteConfirmForm::getDescription() should return Drupal\Core\StringTranslation\TranslatableMarkup but returns string. Currently it just returns the empty string (which works for now, but breaks the core interface contract and could feasibly stop working without warning in a future release). I noticed that the following code block used to exist in 23148a4cfe13fae2fddd49fe66bdaf65de795881:

/**
 * {@inheritdoc}
 * @todo
 */
public function getDescription() {
  $plugin_manager = \Drupal::service('plugin.manager.aws_service');
  $services = $plugin_manager->getDefinitions();
  foreach ($services as $service) {
    $config = $this->configFactory->get('aws.' . $service['id'] . '.settings');
    $profile_id = $config->get('profile');
    if ($profile_id === $this->entity->id()) {
      $affected_services[] = $profile_id;
    }
  }
  return $this->t('@services', ['@services' => $affected_services]);
  // Should return list of services that will be affected.
}

Should that be reinstated? Do we know why it was removed?

User interface changes

API changes

Data model changes

Issue fork aws-3609514

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

andyf created an issue. See original summary.

andyf’s picture

Issue summary: View changes
Status: Active » Needs review

Here's a first go at that.

There's one point where I change the functional behavior: \Drupal\aws\Entity\Form\ProfileForm::save() now returns the correct status.

There's another point where it looks like I change the functional behavior, but it's actually the same: changing AssumeRole to assumeRole in \Drupal\aws\Entity\Profile::getTemporaryCredentials(). This matches the method names declared in the source via @method. It's a safe change: \Aws\AwsClientTrait::__call() calls \Aws\AwsClient::getCommand() which executes ucfirst($name) on the method name before calling it - so we end up dispatching the same call in the end.

andyf’s picture

Issue summary: View changes
andyf’s picture

Issue summary: View changes
Status: Needs review » Postponed

Actually, rather than providing our own phpstan.neon, we can just set the variable _PHPSTAN_LEVEL. Updating IS and MR. Postponing on #3609096: Error when using AWS profiles with an assumed role and STS AssumeRole fails.