GoogleCloudPlatform / marketplace-k8s-app-example

Apache License 2.0
23 stars 19 forks source link

Added REPORTING_SECRET xgoogle-property to wordpress example application. #79

Closed trironkk closed 6 years ago

trironkk commented 6 years ago

This pull request is consistent with our plan of record in the onboarding guide, but it's not clear to me how to retrofit start.sh to leverage this field.

ubbagent needs entitlement-id, consumer-id, and reporting-key, but this x-google-marketplace property only allows for the parameterization of the resource name. Even if that were addressed, the properties passed to the deployer shouldn't include these fields, as they aren't intended to participate in manifest expansion.

I think start.sh should be updated to accept entitlement-id, consumer-id, and reporting-key directly and we should skip adding it to schema.yaml. What y'all you think?

deustis commented 6 years ago

I'm thinking it might be simplest for start.sh to accept the Secret reference, with no knowledge of how to create one. The partner is not able to create a valid entitlement-id without coming to Marketplace. Since we're going to have to generate this asset for them, seems like we might as well generate hand them the fully-formed Secret (same thing as what the customer gets). We would eventually automate this in Partner Portal.

At the end of the day, our console is a privileged actor that can call Procurement to create Entitlements, whereas start.sh is not.

trironkk commented 6 years ago

While having start.sh accept a secret reference is the simplest to implement, it is inconsistent with the interface of the console's Deploy action.

Adjusting start.sh to accept the entitlement-id, consumer-id, and service-account[1] keeps the interfaces consistent, which is valuable for the fidelity of our automated testing and demonstrates the naming semantic that the console's Deploy action will use (naming based on the Application NAME).

There may be reasons create such an inconsistency in the future, but I don't think it's necessary here.

[1] Changed from reporting-key to service-account - I think we should perform the same IAM flow for ServiceAccountKey creation as the console, rather than consume reporting-key data directly because default parameters for start.sh will be source controlled (if not in the partner project, then in our verification scripts), and we shouldn't source control private key data.

trironkk commented 6 years ago

Summarizing offline discussions:

The primary source of misalignment is if start.sh's interface should mirror...

A) the Partner-defined CLI flow by expecting that a reporting credential secret exist on the cluster with a name passed in.

B) the console's Deploy button by consuming the same parameters and constructing the reporting credential secret itself.

This discussion blocks the start.sh changes, but does not need to block this pull request. Could we move forward here?