awslabs / aws-service-catalog-puppet

This is a framework where you list your AWS accounts with tags and your AWS Service Catalog products with tags or target accounts. The framework works through your lists, dedupes and spots collisions and then provisions the products into your AWS accounts for you. It handles the Portfolio sharing, its acceptance and can provision products cross account and cross region.
Apache License 2.0
76 stars 41 forks source link

Share local portfolio to cross region is not working, showing ProductId parameter is missing - i can able to reproduce #723

Open vasindran opened 1 month ago

vasindran commented 1 month ago
          Hi Eamonn, yes i can able to reproduce it.
  1. This is my source account (puppet account) region Frankfurt
image
  1. you can see, there are 11 products in that portfolio
image
  1. This is our puppet manifest file for portfolio share and sharing the portfolio to Franfurt and Ohio (we are running hub mode since we are getting dependency error message when we run in spoke mode)

image

  1. Error message when we run spoke mode
image
  1. Error message reported before image

its just creating empty portfolio, but no products shared

image

Originally posted by @vasindran in https://github.com/awslabs/aws-service-catalog-puppet/issues/722#issuecomment-2379196931

peteawood commented 1 week ago

@eamonnfaherty - This seems to occur if there are products in the hub portfolio which don't exist in the spoke portfolio and either have no versions or all versions are marked to be ignored.

If I'm following the logic correctly, then this line https://github.com/awslabs/aws-service-catalog-puppet/blob/994c221d1fee8efae5fb553f74c80e6ddcd8941a/servicecatalog_puppet/workflow/portfolio/portfolio_management/copy_into_spoke_local_portfolio_task.py#L65 marks the product to be copied and the 'True' value should later on be overwritten with the ProductId at https://github.com/awslabs/aws-service-catalog-puppet/blob/994c221d1fee8efae5fb553f74c80e6ddcd8941a/servicecatalog_puppet/workflow/portfolio/portfolio_management/copy_into_spoke_local_portfolio_task.py#L120-L122


However, if no versions are returned at this line https://github.com/awslabs/aws-service-catalog-puppet/blob/994c221d1fee8efae5fb553f74c80e6ddcd8941a/servicecatalog_puppet/workflow/portfolio/portfolio_management/copy_into_spoke_local_portfolio_task.py#L57 or EVERY version is marked to be ignored at this line https://github.com/awslabs/aws-service-catalog-puppet/blob/994c221d1fee8efae5fb553f74c80e6ddcd8941a/servicecatalog_puppet/workflow/portfolio/portfolio_management/copy_into_spoke_local_portfolio_task.py#L76

then the lines below

https://github.com/awslabs/aws-service-catalog-puppet/blob/994c221d1fee8efae5fb553f74c80e6ddcd8941a/servicecatalog_puppet/workflow/portfolio/portfolio_management/copy_into_spoke_local_portfolio_task.py#L100

https://github.com/awslabs/aws-service-catalog-puppet/blob/994c221d1fee8efae5fb553f74c80e6ddcd8941a/servicecatalog_puppet/workflow/portfolio/portfolio_management/copy_into_spoke_local_portfolio_task.py#L111

will never be true and the "ProductId" of 'True' stored in products_requiring_adding_to_portfolio will remain when the below line finally gets called:

https://github.com/awslabs/aws-service-catalog-puppet/blob/994c221d1fee8efae5fb553f74c80e6ddcd8941a/servicecatalog_puppet/workflow/portfolio/portfolio_management/copy_into_spoke_local_portfolio_task.py#L142-L146

Phew! Apologies for all the snippets, explaining logic flows over gh issues is never fun :) The snippets are from the current version but the same or similar logic existed in v246 (the version in use showing this issue) just on different lines. I'm not sure on the best approach to resolve within puppet but trying to ensure products have at least one relevant version to copy should help resolve when using puppet.

eamonnfaherty commented 1 week ago

Are @vasindran and @peteawood from the same company? From @vasindran report it appears the portfolio has products and they are not marked as ignored but maybe @peteawood has more details?

@vasindran could you email me your log files please.

@peteawood thanks for diving deep into this. Are you affected by this issue also? It was a design decision made back when the project started as portfolios had to have a provisioning artifact.

peteawood commented 1 week ago

Sorry, should have added more context - yes same company.

And yes, the portfolio has products but most of them, in regions other than our primary, have only Version 1 with a name of "-" which I think is what's causing the issue as

https://github.com/awslabs/aws-service-catalog-puppet/blob/29f0b86a4a4afb69b300e8618f3df15cba4c73ea/servicecatalog_puppet/workflow/portfolio/portfolio_management/copy_into_spoke_local_portfolio_task.py#L77-L78

indicates such products, whilst present in the Portfolio, essentially have no valid versions that execute the required logic to replace the 'True' value.

eamonnfaherty commented 1 week ago

Just to clarify - you are unable to share a portfolio that has no valid/active products and you would like to?

peteawood commented 1 week ago

Not quite. Within our primary region, the portfolio's products are all active with valid versions. The portfolio has been copied to our secondary region but (currently) only a subset of the products have valid versions. The products which only have the placeholder version are causing the failure and preventing the active products from being copied to the spoke in the secondary region.

We would like for the portfolio to be shared with the active products even though inactive/invalid products are in the same portfolio. We would expect the invalid/inactive products would not be shared.

However, if that's not possible, or you see that as an undesirable implementation then we could also accept that we simply need to ensure that products should not exist in the portfolio until valid.

The only use case for this I could think of is when products are first created by Factory, I think they appear in a portfolio with just a placeholder version until they are developed further. With a scheduled run of puppet this could prevent sharing working until the first valid version is added.