Skip to content

Commit

Permalink
fix(storage): Fix deserialization regression for storage services (#1041
Browse files Browse the repository at this point in the history
)

* fix deserialization by adding Timestamped and Pipeline mixins classes to objectmappers used in storage services
* add getters/setters for missing Pipeline properties
* update pipeline serialization tests to include additional properties
  • Loading branch information
mochacat committed Jul 9, 2021
1 parent 9ed6b52 commit f9cf126
Show file tree
Hide file tree
Showing 9 changed files with 60 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ public class Pipeline implements Timestamped {
@Setter private String id;
@Getter @Setter private String name;
@Getter @Setter private String application;
@Getter @Setter private Boolean disabled;
@Getter @Setter private String email;
@Getter @Setter private String type;
@Setter private String schema;
Expand All @@ -43,9 +44,15 @@ public class Pipeline implements Timestamped {
@Getter @Setter private Map<String, Object> template;
@Getter @Setter private List<String> roles;
@Getter @Setter private String serviceAccount;
@Getter @Setter private String executionEngine;
@Getter @Setter private Integer stageCounter;
@Getter @Setter private List<Map<String, Object>> stages;
@Getter @Setter private Map<String, Object> constraints;
@Getter @Setter private Map<String, Object> payloadConstraints;
@Getter @Setter private Boolean keepWaitingPipelines;
@Getter @Setter private Boolean limitConcurrent;
@Getter @Setter private List<Map<String, Object>> parameterConfig;
@Getter @Setter private String spelEvaluator;

public void setAny(String key, Object value) {
anyMap.put(key, value);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@
import com.microsoft.azure.storage.StorageException;
import com.microsoft.azure.storage.blob.*;
import com.netflix.spinnaker.front50.api.model.Timestamped;
import com.netflix.spinnaker.front50.api.model.pipeline.Pipeline;
import com.netflix.spinnaker.front50.jackson.mixins.PipelineMixins;
import com.netflix.spinnaker.front50.jackson.mixins.TimestampedMixins;
import com.netflix.spinnaker.kork.web.exceptions.NotFoundException;
import com.netflix.spinnaker.security.AuthenticatedRequest;
import java.io.IOException;
Expand All @@ -41,7 +44,10 @@ public class AzureStorageService implements StorageService {
private CloudStorageAccount storageAccount = null;
private CloudBlobClient blobClient = null;
private CloudBlobContainer blobContainer = null;
private ObjectMapper objectMapper = new ObjectMapper();
private ObjectMapper objectMapper =
new ObjectMapper()
.addMixIn(Timestamped.class, TimestampedMixins.class)
.addMixIn(Pipeline.class, PipelineMixins.class);

private static final String LAST_MODIFIED_FILENAME = "last_modified";
private static final String LAST_MODIFIED_METADATA_NAME = "lastmodifydate";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class PipelineSpec extends Specification {
String pipelineJSON = objectMapper.writeValueAsString(pipeline)

expect:
pipelineJSON == '{"triggers":[],"lastModifiedBy":null,"template":null,"roles":null,"serviceAccount":null,"stages":null,"constraints":null,"payloadConstraints":null,"lastModified":null,"createdAt":null}'
pipelineJSON == '{"disabled":null,"triggers":[],"lastModifiedBy":null,"template":null,"roles":null,"serviceAccount":null,"executionEngine":null,"stageCounter":null,"stages":null,"constraints":null,"payloadConstraints":null,"keepWaitingPipelines":null,"limitConcurrent":null,"parameterConfig":null,"spelEvaluator":null,"lastModified":null,"createdAt":null}'
}

def 'should ignore correct pipeline properties from deserializing JSON to Pipeline'() {
Expand All @@ -45,7 +45,7 @@ class PipelineSpec extends Specification {

def 'roundtrip (JSON -> Pipeline -> JSON) retains arbitrary values'() {
given:
String pipelineJSON = '{"triggers":[],"lastModifiedBy":"anonymous","template":null,"roles":null,"serviceAccount":null,"stages":null,"constraints":null,"payloadConstraints":null,"lastModified":null,"createdAt":null,"foo":"bar"}'
String pipelineJSON = '{"disabled":null,"triggers":[],"lastModifiedBy":"anonymous","template":null,"roles":null,"serviceAccount":null,"executionEngine":null,"stageCounter":null,"stages":null,"constraints":null,"payloadConstraints":null,"keepWaitingPipelines":null,"limitConcurrent":null,"parameterConfig":null,"spelEvaluator":null,"lastModified":null,"createdAt":null,"foo":"bar"}'

String pipeline = objectMapper.writeValueAsString(objectMapper.readValue(pipelineJSON, Pipeline.class))

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@
import com.google.cloud.storage.StorageOptions;
import com.google.common.util.concurrent.ThreadFactoryBuilder;
import com.netflix.spectator.api.Registry;
import com.netflix.spinnaker.front50.api.model.Timestamped;
import com.netflix.spinnaker.front50.api.model.pipeline.Pipeline;
import com.netflix.spinnaker.front50.jackson.mixins.PipelineMixins;
import com.netflix.spinnaker.front50.jackson.mixins.TimestampedMixins;
import com.netflix.spinnaker.front50.model.DefaultObjectKeyLoader;
import com.netflix.spinnaker.front50.model.GcsStorageService;
import com.netflix.spinnaker.front50.model.ObjectKeyLoader;
Expand Down Expand Up @@ -77,7 +81,9 @@ private GcsStorageService googleCloudStorageService(
gcsProperties.getBucketLocation(),
gcsProperties.getRootFolder(),
dataFilename,
new ObjectMapper(),
new ObjectMapper()
.addMixIn(Timestamped.class, TimestampedMixins.class)
.addMixIn(Pipeline.class, PipelineMixins.class),
executor);
log.info(
"Using Google Cloud Storage bucket={} in project={}",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@
package com.netflix.spinnaker.front50.migrations;

import com.fasterxml.jackson.databind.ObjectMapper;
import com.netflix.spinnaker.front50.api.model.Timestamped;
import com.netflix.spinnaker.front50.api.model.pipeline.Pipeline;
import com.netflix.spinnaker.front50.jackson.mixins.PipelineMixins;
import com.netflix.spinnaker.front50.jackson.mixins.TimestampedMixins;
import com.netflix.spinnaker.front50.model.ItemDAO;
import com.netflix.spinnaker.front50.model.pipeline.PipelineDAO;
import com.netflix.spinnaker.front50.model.pipeline.TemplateConfiguration.TemplateSource;
Expand Down Expand Up @@ -49,7 +52,10 @@ public class V2PipelineTemplateSourceToArtifactMigration implements Migration {
public V2PipelineTemplateSourceToArtifactMigration(
PipelineDAO pipelineDAO, ObjectMapper objectMapper) {
this.pipelineDAO = pipelineDAO;
this.objectMapper = objectMapper;
this.objectMapper =
new ObjectMapper()
.addMixIn(Timestamped.class, TimestampedMixins.class)
.addMixIn(Pipeline.class, PipelineMixins.class);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@
import com.fasterxml.jackson.databind.ser.impl.SimpleFilterProvider;
import com.google.common.base.Supplier;
import com.netflix.spinnaker.front50.api.model.Timestamped;
import com.netflix.spinnaker.front50.api.model.pipeline.Pipeline;
import com.netflix.spinnaker.front50.config.OracleProperties;
import com.netflix.spinnaker.front50.jackson.mixins.PipelineMixins;
import com.netflix.spinnaker.front50.jackson.mixins.TimestampedMixins;
import com.netflix.spinnaker.kork.web.exceptions.NotFoundException;
import com.oracle.bmc.auth.AuthenticationDetailsProvider;
import com.oracle.bmc.auth.SimpleAuthenticationDetailsProvider;
Expand Down Expand Up @@ -55,7 +58,10 @@ public class OracleStorageService implements StorageService {
private final String compartmentId;
private final String bucketName;

private final ObjectMapper objectMapper = new ObjectMapper();
private final ObjectMapper objectMapper =
new ObjectMapper()
.addMixIn(Timestamped.class, TimestampedMixins.class)
.addMixIn(Pipeline.class, PipelineMixins.class);

private class RequestSigningFilter extends ClientFilter {
private final RequestSigner signer;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@
import com.fasterxml.jackson.databind.ObjectMapper;
import com.netflix.appinfo.ApplicationInfoManager;
import com.netflix.spectator.api.Registry;
import com.netflix.spinnaker.front50.api.model.Timestamped;
import com.netflix.spinnaker.front50.api.model.pipeline.Pipeline;
import com.netflix.spinnaker.front50.jackson.mixins.PipelineMixins;
import com.netflix.spinnaker.front50.jackson.mixins.TimestampedMixins;
import com.netflix.spinnaker.front50.model.*;
import com.netflix.spinnaker.front50.plugins.PluginBinaryStorageService;
import com.netflix.spinnaker.front50.plugins.S3PluginBinaryStorageService;
Expand Down Expand Up @@ -41,7 +45,10 @@ public AmazonS3 awsS3MetadataClient(
@ConditionalOnProperty(value = "spinnaker.s3.storage-service.enabled", matchIfMissing = true)
public S3StorageService s3StorageService(
AmazonS3 awsS3MetadataClient, S3MetadataStorageProperties s3Properties) {
ObjectMapper awsObjectMapper = new ObjectMapper();
ObjectMapper awsObjectMapper =
new ObjectMapper()
.addMixIn(Timestamped.class, TimestampedMixins.class)
.addMixIn(Pipeline.class, PipelineMixins.class);

S3StorageService service =
new S3StorageService(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.collect.Lists;
import com.netflix.spinnaker.front50.api.model.Timestamped;
import com.netflix.spinnaker.front50.api.model.pipeline.Pipeline;
import com.netflix.spinnaker.front50.jackson.mixins.PipelineMixins;
import com.netflix.spinnaker.front50.jackson.mixins.TimestampedMixins;
import com.netflix.spinnaker.kork.web.exceptions.NotFoundException;
import java.io.ByteArrayInputStream;
import java.io.IOException;
Expand Down Expand Up @@ -59,7 +62,10 @@ public S3StorageService(
Boolean versioning,
Integer maxKeys,
ServerSideEncryption serverSideEncryption) {
this.objectMapper = objectMapper;
this.objectMapper =
new ObjectMapper()
.addMixIn(Timestamped.class, TimestampedMixins.class)
.addMixIn(Pipeline.class, PipelineMixins.class);
this.amazonS3 = amazonS3;
this.bucket = bucket;
this.rootFolder = rootFolder;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@

import com.fasterxml.jackson.databind.ObjectMapper;
import com.netflix.spinnaker.front50.api.model.Timestamped;
import com.netflix.spinnaker.front50.api.model.pipeline.Pipeline;
import com.netflix.spinnaker.front50.jackson.mixins.PipelineMixins;
import com.netflix.spinnaker.front50.jackson.mixins.TimestampedMixins;
import com.netflix.spinnaker.kork.web.exceptions.NotFoundException;
import java.io.ByteArrayInputStream;
import java.io.IOException;
Expand Down Expand Up @@ -51,7 +54,10 @@ public class SwiftStorageService implements StorageService {
private static final Logger log = LoggerFactory.getLogger(SwiftStorageService.class);

private final ObjectStorageService swift;
private final ObjectMapper objectMapper = new ObjectMapper();
private final ObjectMapper objectMapper =
new ObjectMapper()
.addMixIn(Timestamped.class, TimestampedMixins.class)
.addMixIn(Pipeline.class, PipelineMixins.class);
private final String containerName;

private Token token = null;
Expand Down Expand Up @@ -138,7 +144,7 @@ public void deleteObject(ObjectType objectType, String objectKey) {
@Override
public <T extends Timestamped> void storeObject(ObjectType objectType, String objectKey, T item) {
try {
byte[] bytes = new ObjectMapper().writeValueAsBytes(item);
byte[] bytes = objectMapper.writeValueAsBytes(item);
InputStream is = new ByteArrayInputStream(bytes);

getSwift()
Expand Down

0 comments on commit f9cf126

Please sign in to comment.