Skip to content

Commit

Permalink
Merge branch 'handle_collections_with_secrets-2' of https://github.co…
Browse files Browse the repository at this point in the history 
…m/mrdavidlaing/om into mrdavidlaing-handle_collections_with_secrets-2
  • Loading branch information
@kcboyle
kcboyle committed Aug 27, 2020
2 parents 3b51292 + 97e7b72 commit 99a35ac
Show file tree
Hide file tree
Showing 5 changed files with 163 additions and 23 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ can be found in [Pivotal Documentation](docs.pivotal.io/platform-automation).

### Bug Fixes
- `configure-product` will no longer assign a new guid for unnamed collections
- that haven't changed but contain non-configurable properties
that haven't changed but contain non-configurable properties

## 6.1.2

Expand Down
41 changes: 34 additions & 7 deletions api/staged_products_property_collection_guid_assigner.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,20 +79,39 @@ type responsePropertyCollectionItem struct {
Data map[interface{}]interface{}
}

func parseResponsePropertyCollection(rawItems interface{}) (responsePropertyCollection, error) {
func parseResponsePropertyCollection(input associateExistingCollectionGUIDsInput) (responsePropertyCollection, error) {
var collection responsePropertyCollection

rawItemSlice, ok := rawItems.([]interface{})
rawItemSlice, ok := input.ExistingProperty.Value.([]interface{})
if !ok {
return nil, fmt.Errorf("parseResponsePropertyCollection: failed to convert %v to []interface{}", rawItems)
return nil, fmt.Errorf("parseResponsePropertyCollection: failed to convert %v to []interface{}", input.ExistingProperty.Value)
}

for _, item := range rawItemSlice {
for index, item := range rawItemSlice {
itemMap, ok := item.(map[interface{}]interface{})
if !ok {
return nil, fmt.Errorf("parseResponsePropertyCollection: failed to convert %v to map[interface{}]interface{}", item)
}

for collectionItemKey, collectionItemObj := range itemMap {
collectionItemObjAsMap := collectionItemObj.(map[interface{}]interface{})
isCredential := collectionItemObjAsMap["credential"].(bool)
if !isCredential {
continue
}

credentialName := fmt.Sprintf("%s[%d].%s", input.PropertyName, index, collectionItemKey)
apiOutput, err := input.APIService.GetDeployedProductCredential(GetDeployedProductCredentialInput{
DeployedGUID: input.ProductGUID,
CredentialReference: credentialName,
})
if err != nil {
return nil, err
}

collectionItemObjAsMap["value"] = apiOutput.Credential.Value
}

collection = append(collection, responsePropertyCollectionItem{Data: itemMap})
}

Expand Down Expand Up @@ -137,12 +156,20 @@ func (existingCollection responsePropertyCollection) findGUIDForEquivalentlItem(
return "", false
}

func associateExistingCollectionGUIDs(updatedProperty interface{}, existingProperty ResponseProperty) error {
updatedCollection, err := parseUpdatedPropertyCollection(updatedProperty)
type associateExistingCollectionGUIDsInput struct {
APIService Api
ProductGUID string
PropertyName string
UpdatedProperty interface{}
ExistingProperty ResponseProperty
}

func associateExistingCollectionGUIDs(input associateExistingCollectionGUIDsInput) error {
updatedCollection, err := parseUpdatedPropertyCollection(input.UpdatedProperty)
if err != nil {
return err
}
existingCollection, err := parseResponsePropertyCollection(existingProperty.Value)
existingCollection, err := parseResponsePropertyCollection(input)
if err != nil {
return err
}
Expand Down
34 changes: 20 additions & 14 deletions api/staged_products_property_collection_guid_assigner_test.go.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@ import (
)

var _ = Describe("ResponsePropertyCollection", func() {
unmarshalJSONLikeApiGetStagedProductProperties := func(json string) interface{} {
unmarshalJSONLikeApiGetStagedProductProperties := func(json string) ResponseProperty {
var rawCollection interface{}
err := yaml.Unmarshal([]byte(json), &rawCollection)
if err != nil {
panic(fmt.Errorf("Failed to parse json: %w", err))
}
return rawCollection
return ResponseProperty{Value: rawCollection}
}
unmarshalJSON := func(rawJSON string) interface{} {
var rawCollection interface{}
Expand All @@ -28,7 +28,8 @@ var _ = Describe("ResponsePropertyCollection", func() {
}
When("parseResponsePropertyCollection", func() {
It("parses all the elements in the collection", func() {
collection, err := parseResponsePropertyCollection(unmarshalJSONLikeApiGetStagedProductProperties(`[
collection, err := parseResponsePropertyCollection(associateExistingCollectionGUIDsInput{
ExistingProperty: unmarshalJSONLikeApiGetStagedProductProperties(`[
{
"guid": {
"type": "uuid",
Expand Down Expand Up @@ -68,14 +69,15 @@ var _ = Describe("ResponsePropertyCollection", func() {
"optional": false
}
}
]`))
]`)})
Expect(err).To(BeNil())
Expect(len(collection)).To(Equal(2))
})
})
When("extracting field values", func() {
It("correctly extracts guids", func() {
collection, err := parseResponsePropertyCollection(unmarshalJSONLikeApiGetStagedProductProperties(`[
collection, err := parseResponsePropertyCollection(associateExistingCollectionGUIDsInput{
ExistingProperty: unmarshalJSONLikeApiGetStagedProductProperties(`[
{
"guid": {
"type": "uuid",
Expand All @@ -99,12 +101,13 @@ var _ = Describe("ResponsePropertyCollection", func() {
"optional": false
}
}
]`))
]`)})
Expect(err).To(BeNil())
Expect(collection[0].getFieldValue("guid")).To(Equal("28bab1d3-4a4b-48d5-8dac-two"))
})
It("correctly extracts strings", func() {
collection, err := parseResponsePropertyCollection(unmarshalJSONLikeApiGetStagedProductProperties(`[
collection, err := parseResponsePropertyCollection(associateExistingCollectionGUIDsInput{
ExistingProperty: unmarshalJSONLikeApiGetStagedProductProperties(`[
{
"guid": {
"type": "uuid",
Expand All @@ -128,7 +131,7 @@ var _ = Describe("ResponsePropertyCollection", func() {
"optional": false
}
}
]`))
]`)})
Expect(err).To(BeNil())
Expect(collection[0].getFieldValue("name")).To(Equal("the_name"))
})
Expand Down Expand Up @@ -202,7 +205,8 @@ var _ = Describe("ResponsePropertyCollection", func() {
})
When("matching based on item contents", func() {
It("finds items that are identical", func() {
existingCollection, err := parseResponsePropertyCollection(unmarshalJSONLikeApiGetStagedProductProperties(`[
existingCollection, err := parseResponsePropertyCollection(associateExistingCollectionGUIDsInput{
ExistingProperty: unmarshalJSONLikeApiGetStagedProductProperties(`[
{
"guid": {
"type": "uuid",
Expand Down Expand Up @@ -249,7 +253,7 @@ var _ = Describe("ResponsePropertyCollection", func() {
"optional": false
}
}
]`))
]`)})
Expect(err).To(BeNil())
updatedCollection, err := parseUpdatedPropertyCollection(unmarshalJSON(`{ "value":[
{
Expand All @@ -264,7 +268,8 @@ var _ = Describe("ResponsePropertyCollection", func() {
Expect(guid).To(Equal("28bab1d3-4a4b-48d5-8dac-two"))
})
It("finds items that are equivalent but have a different key order", func() {
existingCollection, err := parseResponsePropertyCollection(unmarshalJSONLikeApiGetStagedProductProperties(`[
existingCollection, err := parseResponsePropertyCollection(associateExistingCollectionGUIDsInput{
ExistingProperty: unmarshalJSONLikeApiGetStagedProductProperties(`[
{
"guid": {
"type": "uuid",
Expand All @@ -288,7 +293,7 @@ var _ = Describe("ResponsePropertyCollection", func() {
"optional": false
}
}
]`))
]`)})
Expect(err).To(BeNil())
updatedCollection, err := parseUpdatedPropertyCollection(unmarshalJSON(`{ "value":[
{
Expand All @@ -303,7 +308,8 @@ var _ = Describe("ResponsePropertyCollection", func() {
Expect(guid).To(Equal("28bab1d3-4a4b-48d5-8dac-two"))
})
It("ignores non-configurable properties when finding items that are equivalent", func() {
existingCollection, err := parseResponsePropertyCollection(unmarshalJSONLikeApiGetStagedProductProperties(`[
existingCollection, err := parseResponsePropertyCollection(associateExistingCollectionGUIDsInput{
ExistingProperty: unmarshalJSONLikeApiGetStagedProductProperties(`[
{
"guid": {
"type": "uuid",
Expand All @@ -327,7 +333,7 @@ var _ = Describe("ResponsePropertyCollection", func() {
"optional": false
}
}
]`))
]`)})
Expect(err).To(BeNil())
updatedCollection, err := parseUpdatedPropertyCollection(unmarshalJSON(`{ "value":[
{
Expand Down
7 changes: 6 additions & 1 deletion api/staged_products_service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,12 @@ func (a Api) UpdateStagedProductProperties(input UpdateStagedProductPropertiesIn
for propertyName, property := range newProperties {
configuredProperty := currentConfiguredProperties[propertyName]
if configuredProperty.isCollection() {
err := associateExistingCollectionGUIDs(property, configuredProperty)
err := associateExistingCollectionGUIDs(associateExistingCollectionGUIDsInput{
APIService: a,
ProductGUID: input.GUID,
PropertyName: propertyName,
UpdatedProperty: property,
ExistingProperty: configuredProperty})
if err != nil {
return fmt.Errorf("failed to associate guids for property %#v because:\n%v", propertyName, err)
}
Expand Down
102 changes: 102 additions & 0 deletions api/staged_products_service_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,40 @@ valid options configurations include percentages ('50%'), counts ('2'), and 'def
}],
"optional": false
},
"collection_with_secrets": {
"type": "collection",
"configurable": true,
"credential": false,
"value": [{
"guid": {
"type": "uuid",
"configurable": false,
"credential": false,
"value": "28bab1d3-4a4b-48d5-8dac-with-secret",
"optional": false
},
"super_secret_property": {
"type": "secret",
"configurable": true,
"credential": true,
"value": {
"secret": "***"
},
"optional": false
},
"secret_credentials": {
"type": "simple_credentials",
"configurable": true,
"credential": true,
"value": {
"identity": "unchanged-username",
"password": "***"
},
"optional": true
}
}],
"optional": false
},
"collection_with_name": {
"type": "collection",
"configurable": true,
Expand Down Expand Up @@ -678,6 +712,74 @@ valid options configurations include percentages ('50%'), counts ('2'), and 'def
})
Expect(err).ToNot(HaveOccurred())
})
It("adds the guid for elements with secrets that haven't changed and don't have a logical key field", func() {
client.AppendHandlers(
ghttp.CombineHandlers(
ghttp.VerifyRequest("GET", "/api/v0/deployed/products/some-product-guid/credentials/collection_with_secrets[0].super_secret_property"),
ghttp.RespondWith(http.StatusOK, `{
"credential": {
"type": "secret",
"value": {
"secret": "unchanged_secret"
}
}
}`),
),
ghttp.CombineHandlers(
ghttp.VerifyRequest("GET", "/api/v0/deployed/products/some-product-guid/credentials/collection_with_secrets[0].secret_credentials"),
ghttp.RespondWith(http.StatusOK, `{
"credential": {
"type": "simple_credentials",
"value": {
"identity": "unchanged-username",
"password": "unchanged-password"
}
}
}`),
),
ghttp.CombineHandlers(
ghttp.VerifyRequest("PUT", "/api/v0/staged/products/some-product-guid/properties"),
ghttp.VerifyContentType("application/json"),
ghttp.VerifyJSON(`{
"properties": {
"key": "value",
"collection_with_secrets": {
"value": [{
"super_secret_property": {
"secret": "unchanged_secret"
},
"secret_credentials": {
"identity": "unchanged-username",
"password": "unchanged-password"
},
"guid": "28bab1d3-4a4b-48d5-8dac-with-secret"
}]
}
}
}`),
ghttp.RespondWith(http.StatusOK, `{}`),
),
)

err := service.UpdateStagedProductProperties(api.UpdateStagedProductPropertiesInput{
GUID: "some-product-guid",
Properties: `{
"key": "value",
"collection_with_secrets": {
"value": [{
"super_secret_property": {
"secret": "unchanged_secret"
},
"secret_credentials": {
"identity": "unchanged-username",
"password": "unchanged-password"
}
}]
}
}`,
})
Expect(err).ToNot(HaveOccurred())
})
It("adds the guid for elements that have a name property", func() {
client.AppendHandlers(
ghttp.CombineHandlers(
Expand Down

0 comments on commit 99a35ac

Please sign in to comment.