Skip to content

Feature implementation from commits 984acef..c91d1b4 #3

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 30 commits into
base: feature-base-branch-1
Choose a base branch
from

Conversation

codeOwlAI
Copy link
Owner

@codeOwlAI codeOwlAI commented Jun 29, 2025

PR Summary

Add Kubernetes Website Documentation Generator

Overview

This PR introduces a new documentation generation system for Kubernetes resources. It adds functionality to convert Kubernetes API specifications into website-ready documentation for the Karmada project.

Change Types

Type Description
Feature New documentation generation framework for Kubernetes resources
Feature New CLI command for Kubernetes website specification output
Test Test cases for document generation functionality

Affected Modules

Module / File Change Description
gen-resourcesdocs/cmd/cli/helpers.go Added prepareTOC function for loading TOC configuration
gen-resourcesdocs/cmd/cli/kwebsite.go Added new 'kwebsite' Cobra command for Kubernetes website specs
gen-resourcesdocs/pkg/config/ New files for Chapter structure and documentation output generation
gen-resourcesdocs/pkg/kubernetes/ New components for handling K8s API resources, versions, and groups
gen-resourcesdocs/pkg/outputs/kwebsite/ Implementation of website output components for Hugo
/app/repositories/.../sidebars.js Added documentation reference path for operation scope

Notes for Reviewers

  • The prepareTOC function may need error handling improvement as it doesn't check error before using the returned value
  • The kwebsite implementation appears to be missing some referenced components (escapeName function, addPartIndex method, Part struct)

zhzhuang-zju and others added 30 commits March 17, 2025 20:23
Signed-off-by: zhzhuang-zju <m17799853869@163.com>
add gen-resourcesdocs to generate API docs
Signed-off-by: RainbowMango <qdurenhongcai@gmail.com>
…en_cluster_modes

Explain the difference between Push and Pull mode in details
Signed-off-by: Arhell <arhell333@gmail.com>
Signed-off-by: zhzhuang-zju <m17799853869@163.com>
improve document-releasing process
Signed-off-by: zhzhuang-zju <m17799853869@163.com>
Signed-off-by: 宋阳 <songyang2@xiaohongshu.com>
update install-binary.md to adjust the installation order
Signed-off-by: changzhen <changzhen5@huawei.com>
…or-components

Add binding address configuration for security considerations
Signed-off-by: changzhen <changzhen5@huawei.com>
Signed-off-by: Arhell <arhell333@gmail.com>
Signed-off-by: Arhell <arhell333@gmail.com>
Signed-off-by: zhzhuang-zju <m17799853869@163.com>
add karmadactl operation scope docs
Signed-off-by: changzhen <changzhen5@huawei.com>
add more description for op/cop resourceSelectors
Signed-off-by: wei-chenglai <qazwsx0939059006@gmail.com>
Fix syntax for referencing POD_IP
Signed-off-by: yibing <yibing@dewu.com>
Signed-off-by: chaosi-zju <chaosi@zju.edu.cn>

configDir := cmd.Flag(configDirOption).Value.String()
toc, err := config.LoadTOC(path.Join(configDir, "toc.yaml"))
err = toc.PopulateAssociates(spec)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐛 Correctness Issue

Nil pointer dereference risk.

The code doesn't check the error from LoadTOC before using the toc object, which would cause a runtime panic if toc is nil.

Current Code (Diff):

- 	err = toc.PopulateAssociates(spec)
+ 	if err != nil {
+ 		return nil, err
+ 	}
+ 	err = toc.PopulateAssociates(spec)
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
err = toc.PopulateAssociates(spec)
if err != nil {
return nil, err
}
err = toc.PopulateAssociates(spec)

RunE: func(cmd *cobra.Command, args []string) error {
toc, err := prepareTOC(cmd)
if err != nil {
return fmt.Errorf("Unable to load specs and/or toc config: %v", err)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐛 Correctness Issue

Error Chain Breaking.

The error is not properly wrapped with %w which breaks the error chain and loses context information for debugging.

Current Code (Diff):

- 				return fmt.Errorf("Unable to load specs and/or toc config: %v", err)
+ 				return fmt.Errorf("Unable to load specs and/or toc config: %w", err)
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
return fmt.Errorf("Unable to load specs and/or toc config: %v", err)
return fmt.Errorf("Unable to load specs and/or toc config: %w", err)

newSection = NewSection(o.Name, main, o.Group, o.Version)
} else {
if main = thespec.GetDefinition(o.Key); main == nil {
return fmt.Errorf("Resource %s/%s/%s not found in spec", o.Group, o.Version.String(), kubernetes.APIKind(o.Name))
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐛 Correctness Issue

Incorrect error message format for definition lookup.

The error message incorrectly references Group/Version/Kind format when o.Key is used, which will produce misleading error messages during troubleshooting.

Current Code (Diff):

- 			return fmt.Errorf("Resource %s/%s/%s not found in spec", o.Group, o.Version.String(), kubernetes.APIKind(o.Name))
+ 			return fmt.Errorf("Definition %s not found in spec", o.Key)
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
return fmt.Errorf("Resource %s/%s/%s not found in spec", o.Group, o.Version.String(), kubernetes.APIKind(o.Name))
return fmt.Errorf("Definition %s not found in spec", o.Key)

o.addDefinition(definition, resourceKey, part, toc, thespec)
}
// Mark the resource as documented
thespec.GetResource(*o.Group, *o.Version, kubernetes.APIKind(definition), true)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐛 Correctness Issue

Ignored return value from GetResource call.

The call to GetResource ignores both return values (key and resource), which could mask errors when marking resources as documented.

Current Code (Diff):

- 	thespec.GetResource(*o.Group, *o.Version, kubernetes.APIKind(definition), true)
+ 	key, resource := thespec.GetResource(*o.Group, *o.Version, kubernetes.APIKind(definition), true)
+ 	if resource == nil {
+ 		return
+ 	}
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
thespec.GetResource(*o.Group, *o.Version, kubernetes.APIKind(definition), true)
key, resource := thespec.GetResource(*o.Group, *o.Version, kubernetes.APIKind(definition), true)
if resource == nil {
return
}

func (o *Chapter) searchDefinitionsFromDefinition(suffixes []string, part *Part, toc *TOC, thespec *kubernetes.Spec) {
for _, suffix := range suffixes {
resourceName := o.Name + suffix
resourceKey := kubernetes.Key(o.Key.String() + suffix)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐛 Correctness Issue

Missing separator in key concatenation.

Direct string concatenation for resourceKey may produce invalid keys since o.Key.String() + suffix doesn't include a separator.

Current Code (Diff):

- 		resourceKey := kubernetes.Key(o.Key.String() + suffix)
+ 		resourceKey := kubernetes.Key(o.Key.String() + "." + suffix)
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
resourceKey := kubernetes.Key(o.Key.String() + suffix)
resourceKey := kubernetes.Key(o.Key.String() + "." + suffix)

return err
}

outputChapter, err := outputPart.AddChapter(i, "Common Parameters", "", nil, "", "")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐛 Correctness Issue

Missing error check after AddChapter call.

The code doesn't check the error from AddChapter before using outputChapter, which could cause a nil pointer dereference and application crash.

Current Code (Diff):

- 	outputChapter, err := outputPart.AddChapter(i, "Common Parameters", "", nil, "", "")
+ 	outputChapter, err := outputPart.AddChapter(i, "Common Parameters", "", nil, "", "")
+ 	if err != nil {
+ 		return err
+ 	}
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
outputChapter, err := outputPart.AddChapter(i, "Common Parameters", "", nil, "", "")
outputChapter, err := outputPart.AddChapter(i, "Common Parameters", "", nil, "", "")
if err != nil {
return err
}

if err != nil {
return err
}
err = outputSection.AddContent(kubernetes.ResourcesDescriptions[param][0].Description)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐛 Correctness Issue

Unchecked error from AddContent.

The error returned by outputSection.AddContent is not checked, which could lead to undetected failures and unexpected behavior.

Current Code (Diff):

- 		err = outputSection.AddContent(kubernetes.ResourcesDescriptions[param][0].Description)
+ 		err = outputSection.AddContent(kubernetes.ResourcesDescriptions[param][0].Description)
+ 		if err != nil {
+ 			return err
+ 		}
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
err = outputSection.AddContent(kubernetes.ResourcesDescriptions[param][0].Description)
err = outputSection.AddContent(kubernetes.ResourcesDescriptions[param][0].Description)
if err != nil {
return err
}


// Equals returns true if 'o' and 'p' represent the same version
func (o *APIVersion) Equals(p *APIVersion) bool {
return o.String() == p.String()
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐛 Correctness Issue

Nil pointer dereference in Equals method.

The Equals method doesn't check if parameter 'p' is nil before using it, which could cause a panic at runtime.

Current Code (Diff):

- 	return o.String() == p.String()
+ 	if p == nil {
+ 		return false
+ 	}
+ 	return o.String() == p.String()
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
return o.String() == p.String()
if p == nil {
return false
}
return o.String() == p.String()


// Replaces returns true if 'o' version replaces 'p' version
func (o *APIVersion) Replaces(p *APIVersion) bool {
return o.Version == p.Version && p.LessThan(o)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐛 Correctness Issue

Nil pointer dereference in Replaces method.

The Replaces method doesn't check if parameter 'p' is nil before using it, which could cause a panic at runtime.

Current Code (Diff):

- 	return o.Version == p.Version && p.LessThan(o)
+ 	if p == nil {
+ 		return false
+ 	}
+ 	return o.Version == p.Version && p.LessThan(o)
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
return o.Version == p.Version && p.LessThan(o)
if p == nil {
return false
}
return o.Version == p.Version && p.LessThan(o)

if o.Stage != p.Stage {
return o.Stage < p.Stage
}
return *o.StageVersion < *p.StageVersion
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐛 Correctness Issue

Potential nil pointer dereference when comparing StageVersion.

The code doesn't check if StageVersion pointers are nil before dereferencing them, which could cause a panic if comparing different stage types.

Current Code (Diff):

- 		return *o.StageVersion < *p.StageVersion
+ 		if o.StageVersion == nil || p.StageVersion == nil {
+ 			return o.StageVersion == nil && p.StageVersion != nil
+ 		}
+ 		return *o.StageVersion < *p.StageVersion
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
return *o.StageVersion < *p.StageVersion
if o.StageVersion == nil || p.StageVersion == nil {
return o.StageVersion == nil && p.StageVersion != nil
}
return *o.StageVersion < *p.StageVersion


// AddContent adds content to a section
func (o Section) AddContent(s string) error {
i := len(o.chapter.data.Sections)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐛 Correctness Issue

Potential Index Out-of-Bounds Access.

The code accesses o.chapter.data.Sections[i-1] without checking if i > 0, which could cause a panic if Sections is empty.

Current Code (Diff):

-	i := len(o.chapter.data.Sections)
-	o.chapter.data.Sections[i-1].Description = s
+	if len(o.chapter.data.Sections) > 0 {
+		o.chapter.data.Sections[len(o.chapter.data.Sections)-1].Description = s
+	}
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
i := len(o.chapter.data.Sections)
if len(o.chapter.data.Sections) > 0 {
o.chapter.data.Sections[len(o.chapter.data.Sections)-1].Description = s
}

} else {
fields = &cats[len(cats)-1].Fields
}
j := len(*fields)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐛 Correctness Issue

Potential Index Out-of-Bounds Access.

The code accesses (*fields)[j-1] without checking if j > 0, which could cause a panic if fields is empty.

Current Code (Diff):

-	j := len(*fields)
-	(*fields)[j-1].Type = typ
-	(*fields)[j-1].TypeDefinition = "*" + description + "*"
+	if len(*fields) > 0 {
+		(*fields)[len(*fields)-1].Type = typ
+		(*fields)[len(*fields)-1].TypeDefinition = "*" + description + "*"
+	}
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
j := len(*fields)
if len(*fields) > 0 {
(*fields)[len(*fields)-1].Type = typ
(*fields)[len(*fields)-1].TypeDefinition = "*" + description + "*"
}

}

i = len(o.chapter.data.Sections)
ops := &o.chapter.data.Sections[i-1].Operations
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐛 Correctness Issue

Potential Nil Pointer Dereference.

The code dereferences ops without checking if it's nil, which could cause a panic if o.chapter.data.Sections[i-1].Operations is nil.

Current Code (Diff):

-	*ops = append(*ops, OperationData{
+	if ops == nil {
+		o.chapter.data.Sections[i-1].Operations = []OperationData{}
+		ops = &o.chapter.data.Sections[i-1].Operations
+	}
+	*ops = append(*ops, OperationData{
📝 Committable suggestion

‼️ IMPORTANT
Trust, but verify! 🕵️ Please review this suggestion with the care of a code archaeologist - check that it perfectly replaces the highlighted code, preserves all lines, maintains proper indentation, and won't break anything in production. Your future self will thank you! 🚀

Suggested change
ops := &o.chapter.data.Sections[i-1].Operations
if ops == nil {
o.chapter.data.Sections[i-1].Operations = []OperationData{}
ops = &o.chapter.data.Sections[i-1].Operations
}
*ops = append(*ops, OperationData{

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants