Skip to content
This repository was archived by the owner on Feb 6, 2025. It is now read-only.

fix issue for aws iam profile [bsc#1169506] (1517) #1058

Merged
merged 1 commit into from
Apr 29, 2020

Conversation

cclhsu
Copy link
Contributor

@cclhsu cclhsu commented Apr 23, 2020

Fix issue items for aws iam profile [bsc#1169506]

Signed-off-by: cclhsu clark.hsu@suse.com

Why is this PR needed?

Fix issue items for aws iam profile [bsc#1169506]

Fixes #1517

What does this PR do?

Create aws_iam_role and aws_iam_policy

Anything else a reviewer needs to know?

Special test cases, manual steps, links to resources or anything else that could be helpful to the reviewer.

Info for QA

This is info for QA so that they can validate this. This is mandatory if this PR fixes a bug.
If this is a new feature, a good description in "What does this PR do" may be enough.

Related info

Info that can be relevant for QA:

  • link to other PRs that should be merged together
  • link to packages that should be released together
  • upstream issues

Status BEFORE applying the patch

How can we reproduce the issue? How can we see this issue? Please provide the steps and the prove
this issue is not fixed.

In terraform.tfvar, comment out following:

# iam_profile_master = "caasp-k8s-master-vm-profile"
# iam_profile_worker = "caasp-k8s-worker-vm-profile"

Deploy AWS in non-openbare account will not success.

Status AFTER applying the patch

How can we validate this issue is fixed? Please provide the steps and the prove this issue is fixed.

Deploy AWS in non-openbare account will be successful.

Docs

If docs need to be updated, please add a link to a PR to https://github.com/SUSE/doc-caasp.
At the time of creating the issue, this PR can be work in progress (set its title to [WIP]),
but the documentation needs to be finalized before the PR can be merged.

Merge restrictions

(Please do not edit this)

We are in v4-maintenance phase, so we will restrict what can be merged to prevent unexpected surprises:

What can be merged (merge criteria):
    2 approvals:
        1 developer: code is fine
        1 QA: QA is fine
    there is a PR for updating documentation (or a statement that this is not needed)

@cclhsu cclhsu self-assigned this Apr 23, 2020
Copy link
Contributor

@innobead innobead left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Some suggestions to resolve.

@innobead innobead requested a review from maximenoel8 April 23, 2020 04:00
@c3y1huang
Copy link
Contributor

Is there a test case for this fix?

@innobead
Copy link
Contributor

Is there a test case for this fix?

Good ask ;) @cclhsu please add a test case in caasp-test-cases. cc @maximenoel8

@jenting
Copy link

jenting commented Apr 23, 2020

I don't have an account with permission to create an IAM role to do the test, so personally, I'd prefer let QA testing this PR and approve it.

@cclhsu cclhsu force-pushed the fix_issue_in_aws_iam_profile_1169506 branch 3 times, most recently from 0a0d178 to d4e2eda Compare April 23, 2020 07:30
innobead
innobead previously approved these changes Apr 23, 2020
fix issue items for aws iam profile [bsc#1169506]

Signed-off-by: cclhsu <clark.hsu@suse.com>
Copy link
Member

@flavio flavio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot test it because I do not have a non-openbare account. But it looks good to me

Copy link

@zoopster zoopster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes worked for me as written.

@innobead innobead merged commit 2d746f8 into SUSE:master Apr 29, 2020
@cclhsu cclhsu deleted the fix_issue_in_aws_iam_profile_1169506 branch April 29, 2020 04:25
@cclhsu
Copy link
Contributor Author

cclhsu commented Apr 29, 2020

Have tested after merged. Deployed successfully

@maximenoel8
Copy link

Have tested the deployment with and without iam_profile. It's working fine for me

@cclhsu cclhsu modified the milestone: 4.2.1 May 4, 2020
@cclhsu cclhsu added the 4.2.1 label May 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants