Skip to content

Commit

Permalink
fix(openapi): don't expose private fields in OpenAPI spec
Browse files Browse the repository at this point in the history
Signed-off-by: Marc Nuri <[email protected]>
  • Loading branch information
manusa authored Oct 24, 2024
1 parent 7082932 commit dd4d595
Show file tree
Hide file tree
Showing 15 changed files with 58 additions and 350 deletions.
Original file line number Diff line number Diff line change
@@ -1,16 +1,13 @@

package io.fabric8.knative.pkg.apis;

import java.util.ArrayList;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import javax.annotation.Generated;
import com.fasterxml.jackson.annotation.JsonAnyGetter;
import com.fasterxml.jackson.annotation.JsonAnySetter;
import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.annotation.JsonPropertyOrder;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import io.fabric8.kubernetes.api.builder.Editable;
Expand All @@ -37,8 +34,7 @@
@JsonDeserialize(using = com.fasterxml.jackson.databind.JsonDeserializer.None.class)
@JsonInclude(JsonInclude.Include.NON_NULL)
@JsonPropertyOrder({
"dependents",
"happy"

})
@ToString
@EqualsAndHashCode
Expand All @@ -65,48 +61,9 @@
public class ConditionSet implements Editable<ConditionSetBuilder> , KubernetesResource
{

@JsonProperty("dependents")
@JsonInclude(JsonInclude.Include.NON_EMPTY)
private List<String> dependents = new ArrayList<>();
@JsonProperty("happy")
private String happy;
@JsonIgnore
private Map<String, Object> additionalProperties = new LinkedHashMap<String, Object>();

/**
* No args constructor for use in serialization
*
*/
public ConditionSet() {
}

public ConditionSet(List<String> dependents, String happy) {
super();
this.dependents = dependents;
this.happy = happy;
}

@JsonProperty("dependents")
@JsonInclude(JsonInclude.Include.NON_EMPTY)
public List<String> getDependents() {
return dependents;
}

@JsonProperty("dependents")
public void setDependents(List<String> dependents) {
this.dependents = dependents;
}

@JsonProperty("happy")
public String getHappy() {
return happy;
}

@JsonProperty("happy")
public void setHappy(String happy) {
this.happy = happy;
}

@JsonIgnore
public ConditionSetBuilder edit() {
return new ConditionSetBuilder(this);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,7 @@
"Details",
"Level",
"Message",
"Paths",
"errors"
"Paths"
})
@ToString
@EqualsAndHashCode
Expand Down Expand Up @@ -77,9 +76,6 @@ public class FieldError implements Editable<FieldErrorBuilder> , KubernetesResou
@JsonProperty("Paths")
@JsonInclude(JsonInclude.Include.NON_EMPTY)
private List<String> paths = new ArrayList<>();
@JsonProperty("errors")
@JsonInclude(JsonInclude.Include.NON_EMPTY)
private List<io.fabric8.knative.pkg.apis.FieldError> errors = new ArrayList<>();
@JsonIgnore
private Map<String, Object> additionalProperties = new LinkedHashMap<String, Object>();

Expand All @@ -90,13 +86,12 @@ public class FieldError implements Editable<FieldErrorBuilder> , KubernetesResou
public FieldError() {
}

public FieldError(String details, Integer level, String message, List<String> paths, List<io.fabric8.knative.pkg.apis.FieldError> errors) {
public FieldError(String details, Integer level, String message, List<String> paths) {
super();
this.details = details;
this.level = level;
this.message = message;
this.paths = paths;
this.errors = errors;
}

@JsonProperty("Details")
Expand Down Expand Up @@ -140,17 +135,6 @@ public void setPaths(List<String> paths) {
this.paths = paths;
}

@JsonProperty("errors")
@JsonInclude(JsonInclude.Include.NON_EMPTY)
public List<io.fabric8.knative.pkg.apis.FieldError> getErrors() {
return errors;
}

@JsonProperty("errors")
public void setErrors(List<io.fabric8.knative.pkg.apis.FieldError> errors) {
this.errors = errors;
}

@JsonIgnore
public FieldErrorBuilder edit() {
return new FieldErrorBuilder(this);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import com.fasterxml.jackson.annotation.JsonAnySetter;
import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.annotation.JsonPropertyOrder;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import io.fabric8.kubernetes.api.builder.Editable;
Expand All @@ -35,7 +34,7 @@
@JsonDeserialize(using = com.fasterxml.jackson.databind.JsonDeserializer.None.class)
@JsonInclude(JsonInclude.Include.NON_NULL)
@JsonPropertyOrder({
"s"

})
@ToString
@EqualsAndHashCode
Expand All @@ -62,33 +61,9 @@
public class StatusError implements Editable<StatusErrorBuilder> , KubernetesResource
{

@JsonProperty("s")
private Status s;
@JsonIgnore
private Map<String, Object> additionalProperties = new LinkedHashMap<String, Object>();

/**
* No args constructor for use in serialization
*
*/
public StatusError() {
}

public StatusError(Status s) {
super();
this.s = s;
}

@JsonProperty("s")
public Status getS() {
return s;
}

@JsonProperty("s")
public void setS(Status s) {
this.s = s;
}

@JsonIgnore
public StatusErrorBuilder edit() {
return new StatusErrorBuilder(this);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import com.fasterxml.jackson.annotation.JsonAnySetter;
import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.annotation.JsonPropertyOrder;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import io.fabric8.kubernetes.api.builder.Editable;
Expand All @@ -35,7 +34,7 @@
@JsonDeserialize(using = com.fasterxml.jackson.databind.JsonDeserializer.None.class)
@JsonInclude(JsonInclude.Include.NON_NULL)
@JsonPropertyOrder({
"s"

})
@ToString
@EqualsAndHashCode
Expand All @@ -62,33 +61,9 @@
public class StatusError implements Editable<StatusErrorBuilder> , KubernetesResource
{

@JsonProperty("s")
private Status s;
@JsonIgnore
private Map<String, Object> additionalProperties = new LinkedHashMap<String, Object>();

/**
* No args constructor for use in serialization
*
*/
public StatusError() {
}

public StatusError(Status s) {
super();
this.s = s;
}

@JsonProperty("s")
public Status getS() {
return s;
}

@JsonProperty("s")
public void setS(Status s) {
this.s = s;
}

@JsonIgnore
public StatusErrorBuilder edit() {
return new StatusErrorBuilder(this);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,9 @@ var modules = []module{
{outputName: "openshift-generated", getDefinitionsFunc: generated_openshift_openapi.GetOpenAPIDefinitions, patterns: packages.OpenShiftPackagePatterns},
{outputName: "dev.knative", getDefinitionsFunc: generated_knative_openapi.GetOpenAPIDefinitions, patterns: packages.KnativePackagePatterns},
{outputName: "dev.tekton", getDefinitionsFunc: generated_tekton_openapi.GetOpenAPIDefinitions, patterns: packages.TektonPackagePatterns},
//{outputName: "io.istio", getDefinitionsFunc: generated_istio_openapi.GetOpenAPIDefinitions, patterns: packages.IstioPackagePatterns},
{outputName: "io.k8s.autoscaling", getDefinitionsFunc: generated_autoscaling_openapi.GetOpenAPIDefinitions, patterns: packages.AutoscalingPackagePatterns},
{outputName: "io.k8s.storage.snapshot", getDefinitionsFunc: generated_volumesnapshot_openapi.GetOpenAPIDefinitions, patterns: packages.VolumeSnapshotPackagePatterns},
//{outputName: "io.istio", getDefinitionsFunc: generated_istio_openapi.GetOpenAPIDefinitions, patterns: packages.IstioPackagePatterns},
{outputName: "sh.volcano", getDefinitionsFunc: generated_volcano_openapi.GetOpenAPIDefinitions, patterns: packages.VolcanoPackagePatterns},
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,10 @@ type GoGenerator struct {
func (g *GoGenerator) Generate() error {
g.ReportFilename = g.OutputFile + ".report.txt"
g.processors = []func(context *generator.Context, pkg *types.Package, t *types.Type, member *types.Member, memberIndex int){
processMapKeyTypes,
processOmitPrivateFields,
processPatchComments,
processSwaggerIgnore,
fixVerticalPodAutoscalerInvalidMap,
}
return gengo.Execute(
generators.NameSystems(),
Expand Down Expand Up @@ -152,7 +154,46 @@ func (g *GoGenerator) KubernetesFilterFunc(c *generator.Context, t *types.Type)
return false
}

// Processors used to fix specific issues
////////////////////////////////////////////
// Processors used to fix specific issues //
////////////////////////////////////////////

// processMapKeyTypes function to process the map key types and replace them by string in case they are not
// https://github.com/kubernetes/kube-openapi/blob/67ed5848f094e4cd74f5bdc458cd98f12767c538/pkg/generators/openapi.go#L1062-L1065
// Example errors:
// - failed to generate map property in k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1.HistogramCheckpoint: BucketWeights: map with non-string keys are not supported by OpenAPI in map[int]uint32
// - failed to generate map property in istio.io/api/security/v1beta1.PeerAuthentication: PortLevelMtls: map with non-string keys are not supported by OpenAPI in map[uint32]*istio.io/api/security/v1beta1.PeerAuthentication_MutualTLS
func processMapKeyTypes(context *generator.Context, _ *types.Package, t *types.Type, m *types.Member, memberIndex int) {
if m.Type.Kind == types.Map && m.Type.Key != nil && m.Type.Key.Name.Name != "string" {
t.Members[memberIndex].Type.Key = context.Universe.Type(types.Name{Name: "string"})
}
}

// processOmitPrivateFields
// Ignore private fields by adding the json:"-" tag
func processOmitPrivateFields(_ *generator.Context, _ *types.Package, t *types.Type, m *types.Member, memberIndex int) {
// Skip types that are not exported
if !unicode.IsUpper(rune(m.Name[0])) {
t.Members[memberIndex].Tags = "json:\"-\""
}
}

var patchTags = []string{"patchStrategy", "patchMergeKey"}

// processPatchComments function to process the patchStrategy and patchMergeKey comment tags and add them to the field tags if necessary
// See https://github.com/fabric8io/kubernetes-client/issues/6426#issuecomment-2431653451
func processPatchComments(_ *generator.Context, _ *types.Package, t *types.Type, m *types.Member, memberIndex int) {
for _, patchTag := range patchTags {
tag := reflect.StructTag(m.Tags).Get(patchTag)
if tag != "" {
continue // Value already set, there is no problem
}
tags, ok := gengo.ExtractCommentTags("+", m.CommentLines)[patchTag]
if ok {
t.Members[memberIndex].Tags = t.Members[memberIndex].Tags + " " + patchTag + ":\"" + tags[0] + "\""
}
}
}

// processSwaggerIgnore function to process the swaggerignore tag and add json:omitted so that kube-openapi ignores the field.
func processSwaggerIgnore(_ *generator.Context, _ *types.Package, t *types.Type, m *types.Member, memberIndex int) {
Expand All @@ -166,16 +207,3 @@ func processSwaggerIgnore(_ *generator.Context, _ *types.Package, t *types.Type,
}
}
}

// fixVerticalPodAutoscaler invalidMap key property (replaces it by string)
// failed to generate map property in k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1.HistogramCheckpoint: BucketWeights: map with non-string keys are not supported by OpenAPI in map[int]uint32
// https://github.com/kubernetes/kube-openapi/blob/67ed5848f094e4cd74f5bdc458cd98f12767c538/pkg/generators/openapi.go#L1062-L1065
func fixVerticalPodAutoscalerInvalidMap(context *generator.Context, pkg *types.Package, t *types.Type, m *types.Member, memberIndex int) {
// https://github.com/kubernetes/autoscaler/blob/bb94d270d71795339fc750f1dafeee2a95ed03f5/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1beta1/types.go#L318
// https://github.com/kubernetes/autoscaler/blob/bb94d270d71795339fc750f1dafeee2a95ed03f5/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1beta2/types.go#L347
// https://github.com/kubernetes/autoscaler/blob/bb94d270d71795339fc750f1dafeee2a95ed03f5/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1/types.go#L420C2-L420C15
if m.Name == "BucketWeights" && strings.HasPrefix(pkg.Path, "k8s.io/autoscaler/vertical-pod-autoscaler/") {
stringType := context.Universe.Type(types.Name{Name: "string"})
t.Members[memberIndex].Type.Key = stringType
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -61,16 +61,15 @@ var ChaosMeshPackagePatterns = []string{
}

var IstioPackagePatterns = []string{
"istio.io/api/analysis/v...",
"istio.io/api/meta/v...",
"istio.io/api/security/v...",
"istio.io/api/type/v...",
"istio.io/client-go/pkg/apis/extensions/v...",
"istio.io/api/extensions/v...",
"istio.io/client-go/pkg/apis/networking/v...",
//"istio.io/api/networking/v...",
"istio.io/api/networking/v...",
"istio.io/client-go/pkg/apis/security/v...",
//"istio.io/api/security/v...",
"istio.io/client-go/pkg/apis/telemetry/v...",
"istio.io/api/telemetry/v...",
}

var KnativePackagePatterns = []string{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ var modules = []module{
{patterns: packages.AutoscalingPackagePatterns, outputName: "generated_autoscaling_openapi"},
// https://github.com/chaos-mesh/chaos-mesh/issues/4517
//{patterns: packages.ChaosMeshPackagePatterns, outputName: "generated_chaosmesh_openapi"},
//{patterns: packages.IstioPackagePatterns, outputName: "generated_istio_openapi"},
{patterns: packages.IstioPackagePatterns, outputName: "generated_istio_openapi"},
{patterns: packages.KnativePackagePatterns, outputName: "generated_knative_openapi"},
{patterns: packages.TektonPackagePatterns, outputName: "generated_tekton_openapi"},
{patterns: packages.VolcanoPackagePatterns, outputName: "generated_volcano_openapi"},
Expand Down
27 changes: 1 addition & 26 deletions kubernetes-model-generator/openapi/schemas/dev.knative.json
Original file line number Diff line number Diff line change
Expand Up @@ -7649,23 +7649,6 @@
"dev.knative.pkg.apis.ConditionSet": {
"description": "ConditionSet is an abstract collection of the possible ConditionType values that a particular resource might expose. It also holds the \"happy condition\" for that resource, which we define to be one of Ready or Succeeded depending on whether it is a Living or Batch process respectively.",
"type": "object",
"required": [
"happy",
"dependents"
],
"properties": {
"dependents": {
"type": "array",
"items": {
"type": "string",
"default": ""
}
},
"happy": {
"type": "string",
"default": ""
}
},
"x-fabric8-info": {
"Type": "nested",
"Group": "",
Expand All @@ -7680,8 +7663,7 @@
"required": [
"Message",
"Paths",
"Level",
"errors"
"Level"
],
"properties": {
"Details": {
Expand All @@ -7707,13 +7689,6 @@
"type": "string",
"default": ""
}
},
"errors": {
"type": "array",
"items": {
"default": {},
"$ref": "#/definitions/dev.knative.pkg.apis.FieldError"
}
}
},
"x-fabric8-info": {
Expand Down
Loading

0 comments on commit dd4d595

Please sign in to comment.