search

aws-quickstart / quickstart-ibm-icp-for-data

AWS Quick Start Team
Apache License 2.0
14 stars 19 forks source link

Changes for CPD301 release. #17

Closed shaithal closed 4 years ago

shaithal commented 4 years ago

Issue #, if available:

Description of changes:

Openshift 4.3.x support Changes include support for RH OCS , Portworx as storage along with EFS. Additional services Spark and Cognos Dashboard

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

shaithal commented 4 years ago

Please review @vsnyc . Attached taskcat report taskcat_report.zip

@parthakom2-zz fyi

vsnyc commented 4 years ago

I've made several changes with respect to parameter labels and descriptions. Will share them in a new pull request after merging this in.

For this PR, two main comments:

  1. I saw an access error in the cfn-init logs because the default boot node role does not grant access to the right bucket. The installation did eventually succeed, my guess is because of the AWS admin credentials that get placed later, not completely sure though.

The inline bootnode-policy had the following to allow access to the QSS3Bucket.

 Resource: !Sub 'arn:aws:s3:::${QSS3BucketName}/${QSS3KeyPrefix}*'

It needs to be changed to:

Resource: !Sub
    - arn:${AWS::Partition}:s3:::${S3Bucket}/${QSS3KeyPrefix}*
    - S3Bucket: !If [UsingDefaultBucket, !Sub '${QSS3BucketName}-${AWS::Region}', !Ref QSS3BucketName]
  1. The .taskcat_overrides.yml file shouldn't be checked in. You probably wanted to show that as a template of what's needed to override, but it increases the risk that someone will accidentally commit their secrets to github since the file is included in the source control.

Two additional questions based on reviewing documentation:

  1. APIUsername: should it have a default value of cp? Or should we remove it because it will be obtained by the user?
  2. PortworxSpec: is it always needed or should there be a default value of empty string: ""?
shaithal commented 4 years ago

@vsnyc yes, I will update bootnode policy to look for QSS3Bucket region and get S3Bucket added to Resource arn. APIUsername by default would be cp once we move to IBM Cloud registry after GA, unless any exception or future change. I think we have a documentation reference added for it. @parthakom2-zz please confirm. PortworxSpec is optional and only required when storage is selected as portworx.