Skip to content

Commit

Permalink
fix: order printer columns by json path for consistent ordering
Browse files Browse the repository at this point in the history
Fixes #3040
  • Loading branch information
metacosm authored and manusa committed May 12, 2021
1 parent 5bb0d7a commit a852ecb
Show file tree
Hide file tree
Showing 9 changed files with 150 additions and 42 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
### 5.4-SNAPSHOT

#### Bugs
* Fix #3040: Consistently order printer columns by JSON path to prevent undue changes in generated CRDs
* Fix #3038: Upgrade TLS versions in mock servers to 1.2.
* Fix #3037: Account for JsonProperty annotations when computing properties' name
* Fix #3014: Resync Future is canceled and resync executor is shutdown on informer stop
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import io.fabric8.crd.generator.v1.decorator.EnsureSingleStorageVersionDecorator;
import io.fabric8.crd.generator.v1.decorator.SetServedVersionDecorator;
import io.fabric8.crd.generator.v1.decorator.SetStorageVersionDecorator;
import io.fabric8.crd.generator.v1.decorator.SortPrinterColumnsDecorator;
import io.sundr.codegen.model.TypeDef;
import java.util.Optional;

Expand Down Expand Up @@ -90,4 +91,11 @@ protected void addDecorators(CustomResourceInfo config, TypeDef def,
resources.decorate(new SetStorageVersionDecorator(name, version, config.storage()));
resources.decorate(new EnsureSingleStorageVersionDecorator(name));
}

@Override
public void handle(CustomResourceInfo config) {
super.handle(config);

resources.decorate(new SortPrinterColumnsDecorator());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/**
* Copyright (C) 2015 Red Hat, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.fabric8.crd.generator.v1.decorator;

import io.fabric8.crd.generator.decorator.Decorator;
import io.fabric8.kubernetes.api.model.apiextensions.v1.CustomResourceColumnDefinition;
import io.fabric8.kubernetes.api.model.apiextensions.v1.CustomResourceDefinitionSpecFluent;
import io.fabric8.kubernetes.api.model.apiextensions.v1.CustomResourceDefinitionVersion;
import java.util.List;

public class SortPrinterColumnsDecorator extends Decorator<CustomResourceDefinitionSpecFluent<?>> {

@Override
public void visit(CustomResourceDefinitionSpecFluent<?> spec) {
final List<CustomResourceDefinitionVersion> versions = spec.buildVersions();
for (CustomResourceDefinitionVersion version : versions) {
List<CustomResourceColumnDefinition> columns = version.getAdditionalPrinterColumns();
if(!columns.isEmpty()) {
columns.sort((o1, o2) -> String.CASE_INSENSITIVE_ORDER.compare(o1.getJsonPath(), o2.getJsonPath()));
}
}
spec.withVersions(versions);
}

@Override
public Class<? extends Decorator>[] after() {
return new Class[] {AddAdditionPrinterColumnDecorator.class};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import io.fabric8.crd.generator.v1beta1.decorator.PromoteSingleVersionAttributesDecorator;
import io.fabric8.crd.generator.v1beta1.decorator.SetServedVersionDecorator;
import io.fabric8.crd.generator.v1beta1.decorator.SetStorageVersionDecorator;
import io.fabric8.crd.generator.v1beta1.decorator.SortPrinterColumnsDecorator;
import io.sundr.codegen.model.TypeDef;
import java.util.Optional;

Expand Down Expand Up @@ -91,4 +92,10 @@ protected void addDecorators(CustomResourceInfo config, TypeDef def,
resources.decorate(new EnsureSingleStorageVersionDecorator(name));
resources.decorate(new PromoteSingleVersionAttributesDecorator(name));
}

@Override
public void handle(CustomResourceInfo config) {
super.handle(config);
resources.decorate(new SortPrinterColumnsDecorator());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/**
* Copyright (C) 2015 Red Hat, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.fabric8.crd.generator.v1beta1.decorator;

import io.fabric8.crd.generator.decorator.Decorator;
import io.fabric8.kubernetes.api.model.apiextensions.v1beta1.CustomResourceColumnDefinition;
import io.fabric8.kubernetes.api.model.apiextensions.v1beta1.CustomResourceDefinitionSpecFluent;
import io.fabric8.kubernetes.api.model.apiextensions.v1beta1.CustomResourceDefinitionVersion;
import java.util.List;

public class SortPrinterColumnsDecorator extends Decorator<CustomResourceDefinitionSpecFluent<?>> {

@Override
public void visit(CustomResourceDefinitionSpecFluent<?> spec) {
final List<CustomResourceDefinitionVersion> versions = spec.buildVersions();
for (CustomResourceDefinitionVersion version : versions) {
List<CustomResourceColumnDefinition> columns = version.getAdditionalPrinterColumns();
if(!columns.isEmpty()) {
columns.sort((o1, o2) -> String.CASE_INSENSITIVE_ORDER.compare(o1.getJSONPath(), o2.getJSONPath()));
}
}
spec.withVersions(versions);
}

@Override
public Class<? extends Decorator>[] after() {
return new Class[] {AddAdditionPrinterColumnDecorator.class};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,51 +19,53 @@

public class JokeRequestSpec {

public enum Category {
Any,
Misc,
Programming,
Dark,
Pun,
Spooky,
Christmas
}
public enum Category {
Any,
Misc,
Programming,
Dark,
Pun,
Spooky,
Christmas
}

public enum ExcludedTopic {
nsfw,
religious,
political,
racist,
sexist,
explicit
}
public enum ExcludedTopic {
nsfw,
religious,
political,
racist,
sexist,
explicit
}

@PrinterColumn(name = "jokeCategory")
private Category category = Category.Any;
private ExcludedTopic[] excluded = new ExcludedTopic[] { ExcludedTopic.nsfw, ExcludedTopic.racist, ExcludedTopic.sexist };
private boolean safe;
private Category category = Category.Any;
@PrinterColumn(name = "excludedTopics")
private ExcludedTopic[] excluded = new ExcludedTopic[]{ExcludedTopic.nsfw, ExcludedTopic.racist,
ExcludedTopic.sexist};
private boolean safe;

public Category getCategory() {
return category;
}
public Category getCategory() {
return category;
}

public void setCategory(Category category) {
this.category = category;
}
public void setCategory(Category category) {
this.category = category;
}

public ExcludedTopic[] getExcluded() {
return excluded;
}
public ExcludedTopic[] getExcluded() {
return excluded;
}

public void setExcluded(ExcludedTopic[] excluded) {
this.excluded = excluded;
}
public void setExcluded(ExcludedTopic[] excluded) {
this.excluded = excluded;
}

public boolean isSafe() {
return safe;
}
public boolean isSafe() {
return safe;
}

public void setSafe(boolean safe) {
this.safe = safe;
}
public void setSafe(boolean safe) {
this.safe = safe;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,6 @@ public class Address {
private Type type;

public enum Type {
home, work;
home, work
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,6 @@ public class Person {
private Type type;

public enum Type {
crazy, crazier;
crazy, crazier
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -194,12 +194,18 @@ void jokerequestCRDShouldWork() {

final CustomResourceDefinitionVersion version = checkVersion(spec);
assertNotNull(version.getSubresources());
// printer columns should be ordered in the alphabetical order of their json path
final List<CustomResourceColumnDefinition> printerColumns = version
.getAdditionalPrinterColumns();
assertEquals(1, printerColumns.size());
final CustomResourceColumnDefinition columnDefinition = printerColumns.get(0);
assertEquals(2, printerColumns.size());
CustomResourceColumnDefinition columnDefinition = printerColumns.get(0);
assertEquals("string", columnDefinition.getType());
assertEquals(".spec.category", columnDefinition.getJsonPath());
assertEquals("jokeCategory", columnDefinition.getName());
columnDefinition = printerColumns.get(1);
assertEquals("string", columnDefinition.getType());
assertEquals(".spec.excluded", columnDefinition.getJsonPath());
assertEquals("excludedTopics", columnDefinition.getName());
CustomResourceValidation schema = version.getSchema();
assertNotNull(schema);
Map<String, JSONSchemaProps> properties = schema.getOpenAPIV3Schema().getProperties();
Expand Down

0 comments on commit a852ecb

Please sign in to comment.