fix(openapi): don't expose private fields in OpenAPI spec

Signed-off-by: Marc Nuri <[email protected]>
fabric8io · Oct 24, 2024 · 88e31dc · 88e31dc
1 parent b6bcc3d
commit 88e31dc
Show file tree

Hide file tree

Showing 22 changed files with 102 additions and 357 deletions.
diff --git a/doc/quarkus-defaults.md b/doc/quarkus-defaults.md
@@ -0,0 +1,34 @@
+Starting from the xxxx version, the default values for the Kubernetes Client extension configuration have changed.
+In the past, Quarkus had explicit values for some of the Kubernetes Client configuration options.
+This was not ideal since some of these default settings were different to those provided by the client library which led to some confusion.
+To address this, we have removed the explicit default values from the Kubernetes Client extension configuration and now rely on the client library defaults.
+
+The following table contains all the configuration option entries, the previous default value, and the new default value (note that many of the values remain unchanged and others don't have a default value):
+
+| Configuration Option          | Previous Default Value | New Default Value |
+|-------------------------------|------------------------|-------------------|
+| `watchReconnectInterval`      | `PT1S`                 | `PT1S`            |
+| `watchReconnectLimit`         | `-1` (no limit)        | `-1` (no limit)   |
+| `connectionTimeout`           | `PT10S`                | `PT10S`           |
+| `requestTimeout`              | `PT10S`                | `PT10S`           |
+| `requestRetryBackoffLimit`    | `0` (no retry)         | `10`              |
+| `requestRetryBackoffInterval` | `PT1S`                 | `PT0.1S`          |
+| `trustCerts`                  | n/a                    | `false`           |
+| `apiServerUrl`                | n/a                    | n/a               |
+| `namespace`                   | n/a                    | n/a               |
+| `caCertFile`                  | n/a                    | n/a               |
+| `caCertData`                  | n/a                    | n/a               |
+| `clientCertFile`              | n/a                    | n/a               |
+| `clientCertData`              | n/a                    | n/a               |
+| `clientKeyFile`               | n/a                    | n/a               |
+| `clientKeyData`               | n/a                    | n/a               |
+| `clientKeyAlgo`               | n/a                    | `RSA`             |
+| `clientKeyPassphrase`         | n/a                    | n/a               |
+| `username`                    | n/a                    | n/a               |
+| `password`                    | n/a                    | n/a               |
+| `token`                       | n/a                    | n/a               |
+| `httpProxy`                   | n/a                    | n/a               |
+| `httpsProxy`                  | n/a                    | n/a               |
+| `proxyUsername`               | n/a                    | n/a               |
+| `proxyPassword`               | n/a                    | n/a               |
+| `noProxy`                     | n/a                    | n/a               |
diff --git a/extensions/knative/model/src/generated/java/io/fabric8/knative/pkg/apis/ConditionSet.java b/extensions/knative/model/src/generated/java/io/fabric8/knative/pkg/apis/ConditionSet.java
@@ -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;
@@ -37,8 +34,7 @@
 @JsonDeserialize(using = com.fasterxml.jackson.databind.JsonDeserializer.None.class)
 @JsonInclude(JsonInclude.Include.NON_NULL)
 @JsonPropertyOrder({
-    "dependents",
-    "happy"
+
 })
 @ToString
 @EqualsAndHashCode
@@ -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);

diff --git a/extensions/knative/model/src/generated/java/io/fabric8/knative/pkg/apis/FieldError.java b/extensions/knative/model/src/generated/java/io/fabric8/knative/pkg/apis/FieldError.java
@@ -40,8 +40,7 @@
     "Details",
     "Level",
     "Message",
-    "Paths",
-    "errors"
+    "Paths"
 })
 @ToString
 @EqualsAndHashCode
@@ -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>();
 
@@ -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")
@@ -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);

diff --git a/...ions/tekton/model/src/generated/java/io/fabric8/tekton/triggers/v1alpha1/StatusError.java b/...ions/tekton/model/src/generated/java/io/fabric8/tekton/triggers/v1alpha1/StatusError.java
@@ -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;
@@ -35,7 +34,7 @@
 @JsonDeserialize(using = com.fasterxml.jackson.databind.JsonDeserializer.None.class)
 @JsonInclude(JsonInclude.Include.NON_NULL)
 @JsonPropertyOrder({
-    "s"
+
 })
 @ToString
 @EqualsAndHashCode
@@ -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);

diff --git a/...sions/tekton/model/src/generated/java/io/fabric8/tekton/triggers/v1beta1/StatusError.java b/...sions/tekton/model/src/generated/java/io/fabric8/tekton/triggers/v1beta1/StatusError.java
@@ -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;
@@ -35,7 +34,7 @@
 @JsonDeserialize(using = com.fasterxml.jackson.databind.JsonDeserializer.None.class)
 @JsonInclude(JsonInclude.Include.NON_NULL)
 @JsonPropertyOrder({
-    "s"
+
 })
 @ToString
 @EqualsAndHashCode
@@ -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);

diff --git a/kubernetes-model-generator/openapi/generator/cmd/openapi.go b/kubernetes-model-generator/openapi/generator/cmd/openapi.go
@@ -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},
 }
 

diff --git a/kubernetes-model-generator/openapi/generator/pkg/openapi/openapi-gen.go b/kubernetes-model-generator/openapi/generator/pkg/openapi/openapi-gen.go
@@ -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(),
@@ -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) {
@@ -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
-	}
-}
diff --git a/kubernetes-model-generator/openapi/generator/pkg/packages/packages.go b/kubernetes-model-generator/openapi/generator/pkg/packages/packages.go
@@ -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{

diff --git a/kubernetes-model-generator/openapi/generator/tools/generator/openapi.go b/kubernetes-model-generator/openapi/generator/tools/generator/openapi.go
@@ -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"},

diff --git a/...napi/maven-plugin/src/main/java/io/fabric8/kubernetes/schema/generator/ImportManager.java b/...napi/maven-plugin/src/main/java/io/fabric8/kubernetes/schema/generator/ImportManager.java
@@ -26,9 +26,14 @@ public interface ImportManager {
 
   default void addImport(String importedClass) {
     // Only add import if it belongs to a different package
-    if (!Objects.equals(importedClass.substring(0, importedClass.lastIndexOf('.')), getPackageName())) {
-      getImports().add(importedClass);
+    if (Objects.equals(importedClass.substring(0, importedClass.lastIndexOf('.')), getPackageName())) {
+      return;
     }
+    // Do not import classes from java.lang
+    if (importedClass.substring(0, importedClass.lastIndexOf('.')).equals("java.lang")) {
+      return;
+    }
+    getImports().add(importedClass);
   }
 
   default void addAllImports(Collection<String> allImports) {